Fixes #36691 - use 'connectefi scsi' by default

[sbernhard]

[theforeman-bot]

Issues: #36691

[sbernhard]

[test unit]

[sbernhard]

Failed tests are unrelated to this PR.

[sbernhard]

Many tests failed but I think they are not related to this PR.

[jloeser]

JFYI: PR https://github.com/theforeman/foreman/pull/10126 could make this connectefi scsi approach obsolete.

If https://github.com/theforeman/foreman/pull/9864 gets merged, this will probably not work anymore because most distributions don't have GRUB2 connectefi module installed.

I would make connectefi scsi opt-in.

[jloeser]

There was the idea from @goarsna to let GRUB2 find out if connectefi module is supported during runtime. With lsmod we can see if a module is available (or not). But I couldn't find anything quickly parse output and end up with a if-condition.

[jloeser]

Having connectefi module not available and running the command does not harm (just an unknown command):

Failed to find fs: Unsupported
Fetching Netboot Image
error: ../../grub-core/script/function.c:119:can't find command `connectefi'.
next instruction ...
SecureBoot enabled?
y

GRUB config:

connectefi scsi
echo "next instruction ..."
echo "SecureBoot enabled?"
echo $lockdown
sleep 10

Means if we enable it by default and the module ...

• is available, chainloading would work as expected
• is not available, we end up as before with this error message - which doesn't hurt

[sbernhard]

Looks like the tests are green. The other option with truthy did not work because the value can be scsi or pciroot, too

[ekohl]

Having connectefi module not available and running the command does not harm (just an unknown command):

I get the impression our CI disagrees. It says Invalid command and fails with Out of Memory on a grub check. So either the grub check is incorrect, or it breaks on non-patched grub.

[sbernhard]

Thank you very much @ekohl and grub-script-check Forget a a closing " (double-quote).

17:38 $ LANG=C grub-script-check unit/foreman/renderer/snapshots/ProvisioningTemplate/PXEGrub2/PXEGrub2_default_local_boot.host4dhcp.snap.txt error: out of memory. error: syntax error. error: Incorrect command. error: syntax error. Syntax error at line 223

[stejskalleos]

All tests are green, @ekohl if you okay I'm okay to merge

[ekohl]

Yay for CI. Verifying snapshots with external validator tools has proven useful.

[Lennonka]

I'm a bit worried this might not work as expected for another than global host parameter if any.

To my knowledge, the including templates (global default and local boot) are rendered once and the same config is deployed to all SmartProxies (AKA the Build PXE default button). In other words, the scope in which the snippet is rendered, is not a host, but it's the Foreman server itself. Therefore, the templates are indifferent towards hosts, subnets, organizations..., I would expect. And if that's true, the unit test doesn't represent a valid test case and its results are inconclusive.

I'd be more than happy to be proven wrong.

[jloeser]

To my knowledge, the including templates (global default and local boot) are rendered once and the same config is deployed to all SmartProxies (AKA the Build PXE default button). In other words, the scope in which the snippet is rendered, is not a host, but it's the Foreman server itself.

Partly true. The "local boot" template is host scoped and gets rendered whenever a host's build state changes.

If you render non-host scoped global template (Build PXE default button), the connectefi command is absent. In this case it can happen that hosts cannot chainload from disk if they are not managed by Foreman (means there is no local boot template). Then one would need to set connectefi scsi manually (as before).

Two things here:

1. Executing connectefi scsi does not harm, even if the command does not exist in GRUB2; in the best case, hosts will boot up successfully, or not (as before)
2. We plan to get rid of chainloader command at all which would make this connectefi command completely obsolete. Idea is to simply load the configuration file (see https://github.com/theforeman/foreman/pull/10207)

[Lennonka]

Thank you so much for the clarification