Skip to content

Conversation

chcao
Copy link
Contributor

@chcao chcao commented Aug 21, 2025

@chcao chcao force-pushed the add_kernel_parameters_to_unattended_profile branch 2 times, most recently from 2d6612b to 9ab6b37 Compare August 22, 2025 06:53
@jknphy
Copy link
Contributor

jknphy commented Aug 22, 2025

Please, rebase, there has been changes recently for bootloader timeout.
For this parameter, now we have the challengue that can go along a grub timeout stop or a grub timeout, but not both.
Is in progress: https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/22904/files

@chcao chcao force-pushed the add_kernel_parameters_to_unattended_profile branch from 9ab6b37 to 4559d8f Compare August 25, 2025 07:46
@chcao chcao force-pushed the add_kernel_parameters_to_unattended_profile branch 6 times, most recently from 42d8c3c to 08e35c6 Compare August 27, 2025 13:06
@chcao chcao added the qe-yam label Aug 28, 2025
@chcao chcao changed the title [WIP] Add kernel parameters to bootloader section Add kernel parameters to bootloader section Aug 28, 2025
@chcao chcao force-pushed the add_kernel_parameters_to_unattended_profile branch from 08e35c6 to 7eb1101 Compare August 28, 2025 12:22
Copy link
Contributor

@rakoenig rakoenig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine.

@chcao chcao force-pushed the add_kernel_parameters_to_unattended_profile branch 8 times, most recently from d78371f to 0ec20e5 Compare September 2, 2025 09:08
@chcao chcao force-pushed the add_kernel_parameters_to_unattended_profile branch from 0ec20e5 to c7555ea Compare September 3, 2025 05:49
Copy link
Contributor

@manfredi manfredi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@jknphy
Copy link
Contributor

jknphy commented Sep 3, 2025

I have been discussing with @okynos different solutions, he will share with you regarding to have more compact libraries and not spreading value (not even keys) in the template.

Copy link
Contributor

@okynos okynos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion, discuss with @jknphy

@jknphy
Copy link
Contributor

jknphy commented Sep 4, 2025

we should test further this, but a solution to have single function is this:
base.libsonnet:

{
  bootloader(bootloader, bootloader_timeout, bootloader_extra_kernel_params):: {
    [if bootloader || bootloader_timeout != '' || bootloader_extra_kernel_params != '' then 'bootloader']: std.prune({
      [if bootloader then 'stopOnBootMenu']: true,
      [if bootloader_timeout then 'timeout']: 15,
      [if bootloader_extra_kernel_params != '' then 'extraKernelParams']: bootloader_extra_kernel_params,
    }),
  },
  files: [{
    destination: '/usr/local/share/dummy.xml',
    url: 'dummy.xml',
  }],
  ...

template.libsonnet:

   ...
   storage='',
   user=true) (
  base_lib.bootloader(bootloader, bootloader_timeout, bootloader_extra_kernel_params) +
  {
    [if dasd == true then 'dasd']: dasd_lib.dasd(),
    ...
      }
)

Notice (, ) and + to make possible to use function directly in the function and merge it with the other keys.

@chcao chcao force-pushed the add_kernel_parameters_to_unattended_profile branch 4 times, most recently from 3d7ae0c to 568647b Compare September 5, 2025 05:22
Add kernel parameters to bootloader section.
@chcao chcao force-pushed the add_kernel_parameters_to_unattended_profile branch from 568647b to 3154479 Compare September 5, 2025 05:53
sub run {
select_console 'root-console';
my $output = script_output('cat /proc/cmdline');
my ($parameters) = get_var('AGAMA_PROFILE_OPTIONS') =~ /bootloader_extra_kernel_params="([^"]*)"/;
Copy link
Contributor

@jknphy jknphy Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With a named regex without escaping the quotes twice (as you are escaping it after the equal and at the end):
get_var('AGAMA_PROFILE_OPTIONS', '') =~ /bootloader_extra_kernel_params="(?<kernel_params>.*)"/;

And then use it with:
$+{kernel_params}

my $output = script_output('cat /proc/cmdline');
my ($parameters) = get_var('AGAMA_PROFILE_OPTIONS') =~ /bootloader_extra_kernel_params="([^"]*)"/;

unless ($output =~ /\Q$parameters\E/) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need \Q and \E, might be too much I think.
it would be read more naturally in one line:
die "$parameters not found in kernel command line!" unless $output =~ /$+{kernel_params}/);

sub run {
select_console 'root-console';
my $output = script_output('cat /proc/cmdline');
my ($parameters) = get_var('AGAMA_PROFILE_OPTIONS') =~ /bootloader_extra_kernel_params="([^"]*)"/;
Copy link
Contributor

@jknphy jknphy Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this operation is a parsing, it should go before running api call, like script_output, it is kind of an initialization (by extraction), although select_console might be an exception, you could put it after select_console, one line above.

@@ -8,6 +8,7 @@ schedule:
- yam/agama/agama
- boot/reconnect_mgmt_console
- installation/first_boot
- yam/validate/validate_extra_kernel_params
Copy link
Contributor

@jknphy jknphy Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure where I have seen this before, but please add your check on the bottom always, not on top, only for your debugging you could have it there while the PR is in WIP. It is not always the same rule, but when we are talking about specific checks, it applies, if we were doing some really generic check we would like to have it right after booting probably.
Additionally in theory it should match the order here: https://gitlab.suse.de/qe-yam/openqa-job-groups/-/merge_requests/563/diffs#5d8fe6da111ccf64f032b338d77178a533febb54_50_50 so it could easy guide us where things are.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants