(In reply to Christophe Besson from comment #0)
> Description of problem:
> 
> Using grub2-mkconfig removes extra args in $kernel_opts
> (/boot/grub2/grubenv). This is because grub2-mkconfig updates the cmdline
> from $GRUB_CMDLINE_LINUX (/etc/default/grub). 
> 
> 2 ways to define the cmdline may lead to an unexpected behavior.
> 
> 
> Version-Release number of selected component (if applicable):
> grub2-tools-2.02-66.el8
> 
> 
> How reproducible:
> 
> 
> Steps to Reproduce:
> 1. # grub2-editenv - set kernelopts="root=/dev/mapper/rhel-root ro
> crashkernel=auto resume=/dev/mapper/rhel-swap rd.lvm.lv=rhel/root
> rd.lvm.lv=rhel/swap rhgb quiet rd.debug"
> 
> 2. # grub2-mkconfig
> 
> 3. # grub2-editenv - list | grep kern
> kernelopts=root=/dev/mapper/rhel-root ro crashkernel=auto
> resume=/dev/mapper/rhel-swap rd.lvm.lv=rhel/root rd.lvm.lv=rhel/swap rhgb
> quiet
> 
> 
> Actual results:
> The extra arg "rd.debug" has been removed from $kernel_opts.
> 
> Expected results:
> Keep the extra args added through grub2-editenv
>

It used to behave that way but was changed because most users expect the opposite. That is, to update the kernel cmdline by setting GRUB_CMDLINE_LINUX in /etc/default/grub and running grub2-mkconfig. See Bug #1637875.

> Additional info:
> A side effect occurs in the sosreport. This tool is not supposed to change
> the customer configuration. It can be workarounded by adding:
> --no-grubenv-update to the grub2.py plugin which invokes grub2-mkconfig
> (line 37).

Yes, that's why this option was added.

I changed the component involved to "sos", since the old way to configure the cmdline is finally the preferred one. sos should not modify the customer configuration.

Just to confirm if I got it right:

1) sosreport calls "grub2-mkconfig" that can alter args in $kernel_opts (/boot/grub2/grubenv)
2) to prevent that, sosreport should call the command with --no-grubenv-update option
3) using OS environment variable GRUB_DISABLE_OS_PROBER has no effect any time (that was used to prevent the changes)
4) there is no easy way to detect what grub2-mconfig version supports --no-grubenv-update option (i.e. grub2-tools-2.02-66.el8.x86_64 does support it, while grub2-tools-2.02-0.76.el7.1.x86_64 does not)
  - so sosreport should call "grub2-mconfig --no-grubenv-update" first, and only if it fails, it will call "grub2-mconfig" without an argument


Am I right here (esp. in the statements 3) and 4) )?

(In reply to Pavel Moravec from comment #3)
> Just to confirm if I got it right:
> 
> 1) sosreport calls "grub2-mkconfig" that can alter args in $kernel_opts
> (/boot/grub2/grubenv)
> 2) to prevent that, sosreport should call the command with
> --no-grubenv-update option

Correct.

> 3) using OS environment variable GRUB_DISABLE_OS_PROBER has no effect any
> time (that was used to prevent the changes)

That's different. Is to prevent the os-prober tool to be executed, which is used to probe for other installed operating systems.

> 4) there is no easy way to detect what grub2-mconfig version supports
> --no-grubenv-update option (i.e. grub2-tools-2.02-66.el8.x86_64 does support
> it, while grub2-tools-2.02-0.76.el7.1.x86_64 does not)
>   - so sosreport should call "grub2-mconfig --no-grubenv-update" first, and
> only if it fails, it will call "grub2-mconfig" without an argument
> 
>

Another option is check the exit status of grub2-mkconfig --help | grep "no-grubenv-update", to figure whether the option is supported or not.
 
> Am I right here (esp. in the statements 3) and 4) )?

> 3) using OS environment variable GRUB_DISABLE_OS_PROBER has no effect any time (that was used to prevent the changes)

Yeah. That's completely unrelated. That prevents us mucking around with loaded kernel file system modules when probing other OSs on the host. This is to prevent re-sync of the grub2 environment from the filesystem config files. Trivial change just needs to go upstream in sos.

We could extend the predicate mechanism to search for switches in --help output... that would be one way to gate this for grub2 versions that lack it. Other obvious but ugly way would be to make the change RHEL8 specific, but I'm not keen on that.

> > 4) there is no easy way to detect what grub2-mconfig version supports
> > --no-grubenv-update option (i.e. grub2-tools-2.02-66.el8.x86_64 does support
> > it, while grub2-tools-2.02-0.76.el7.1.x86_64 does not)
> >   - so sosreport should call "grub2-mconfig --no-grubenv-update" first, and
> > only if it fails, it will call "grub2-mconfig" without an argument
> > 
> >
> 
> Another option is check the exit status of grub2-mkconfig --help | grep
> "no-grubenv-update", to figure whether the option is supported or not.
>  

I meant to write grep -q here.

Another option is to grep the --help in python (I dont like much calling external programs with pipe), like:

diff --git a/sos/plugins/grub2.py b/sos/plugins/grub2.py
index 9786de44..89911ca8 100644
--- a/sos/plugins/grub2.py
+++ b/sos/plugins/grub2.py
@@ -32,9 +32,17 @@ class Grub2(Plugin, RedHatPlugin, DebianPlugin, UbuntuPlugin):
         self.add_cmd_output("ls -lanR /boot")
         # call grub2-mkconfig with GRUB_DISABLE_OS_PROBER=true to prevent
         # possible unwanted loading of some kernel modules
+        # further, check if the command supports --no-grubenv-update option
+        # to prevent removing of extra args in $kernel_opts, and (only) if so,
+        # call the command with this argument
         env = {}
         env['GRUB_DISABLE_OS_PROBER'] = 'true'
-        self.add_cmd_output("grub2-mkconfig", env=env)
+        cmd = "grub2-mkconfig"
+        for lines in self.call_ext_prog("grub2-mkconfig --help")['output'].splitlines():
+            if "--no-grubenv-update" in lines:
+                cmd += " --no-grubenv-update"
+                break
+        self.add_cmd_output(cmd, env=env)
 
     def postproc(self):
         # the trailing space is required; python treats '_' as whitespace

Upstream PR proposed, based on #c8.

I think I would rather see this as a new predicate type: it's not the first time we've had this situation, and we've solved it with this kind of ad-hoc glue before like version checking. It'd be far better to be able to write things like:

  self.add_cmd_output("grub2-mkconfig --no-grubenv-update", pred=SoSPredicate(self, args=["--no-grubenv-update"]))

As it documents why we're doing this and it will work regardless of version values, backports etc.

deferring to 8.2 due to the pending upstream PR and no QE capacity

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHEA-2020:1900