Skip to content
This repository has been archived by the owner on Jul 9, 2020. It is now read-only.

Fixes #29952 - make Grub2 template bootdisk-efi compatible #738

Merged
merged 1 commit into from Jun 5, 2020

Conversation

lzap
Copy link
Member

@lzap lzap commented May 28, 2020

The BOOTIF statement breaks the full host disk. It cannot be set to BOOTIF=01-$mac_default_net because when not PXE/HTTP booting, this variable is NOT set in grub, therefore it renders to BOOTIF=01- which is wrong and dracut cannot initialize the network.

Do not merge yet, testing with @adamruzicka for the latest bootdisk UEFI changes.

@@ -11,6 +11,13 @@ oses:
<%
rhel_compatible = @host.operatingsystem.family == 'Redhat' && @host.operatingsystem.name != 'Fedora'
os_major = @host.operatingsystem.major.to_i
# Grub2 does not have IPAPEND feature
if @host.provition_interface.mac
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if @host.provition_interface.mac
if @host.provision_interface.mac

Copy link
Member Author

Choose a reason for hiding this comment

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

I am writing this all the time.

@@ -11,6 +11,13 @@ oses:
<%
rhel_compatible = @host.operatingsystem.family == 'Redhat' && @host.operatingsystem.name != 'Fedora'
os_major = @host.operatingsystem.major.to_i
# Grub2 does not have IPAPEND feature
if @host.provition_interface.mac
bootif = "01-#{@host.provition_interface.mac.tr(':','-')}"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
bootif = "01-#{@host.provition_interface.mac.tr(':','-')}"
bootif = "01-#{@host.provision_interface.mac.tr(':','-')}"

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah muscle memory.

Copy link
Contributor

@adamruzicka adamruzicka left a comment

Choose a reason for hiding this comment

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

Added some comments in addition to what Timo said

@@ -26,13 +33,13 @@ set default=0
set timeout=<%= host_param('loader_timeout') || 10 %>

menuentry '<%= template_name %>' {
<%= linuxcmd %> <%= @kernel %> ks=<%= foreman_url('provision') %> <%= pxe_kernel_options %> <%= snippet("kickstart_kernel_options").strip %> BOOTIF=01-$net_default_mac
<%= linuxcmd %> <%= @kernel %> ks=<%= foreman_url('provision') %> <%= pxe_kernel_options %> <%= snippet("kickstart_kernel_options").strip %> BOOTIF=<%= bootif %>
Copy link
Contributor

Choose a reason for hiding this comment

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

This renders the following piece on my testing machine

-----B<-----SNIP-----B<-----
menuentry 'Kickstart default PXEGrub2 @ b3d7bae' {
linux boot/centos-mirror-gTpaQO8OPF0S-vmlinuz ks=http://192.168.122.48/unattended/provision?token=294e47d3-4cf9-48f3-8b08-00d53c973b63  network ksdevice=bootif ks.device=bootif BOOTIF=00-52-54-00-11-11-11 kssendmac ks.sendmac inst.ks.sendmac ip=dhcp BOOTIF=01-52-54-00-11-11-11
  initrd boot/centos-mirror-gTpaQO8OPF0S-initrd.img
}
-----B<-----SNIP-----B<-----

If the same parameter (BOOTIF in this case) is provided, the later one "wins"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, it's actually already defined I am going to drop this. The variable does not need to be used at all, that's for discovery case which is a different template not this one.

else
# this only works for PXE/HTTP UEFI boot
bootif = "01-$net_default_mac"
end

if (rhel_compatible && os_major < 7) || !@host.pxe_loader_efi?
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried provisioning CentOS 7.7.1908 and got linux and initrd as commands, however when booting from the bootdisk image in UEFI mode, grub complained about

error: can't find command 'linux'.
error: can't find command 'initrd'.

Changing those by hand to linuxefi and initrdefi made the issue go away

Copy link
Member Author

Choose a reason for hiding this comment

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

Your PXE loader must include "EFI" string, otherwise it's incorrectly set here.

@lzap lzap force-pushed the bootif-bootdisk-fix-29952 branch 2 times, most recently from 4c51139 to 43d47e4 Compare June 1, 2020 12:34
@ekohl
Copy link
Member

ekohl commented Jun 1, 2020

It feels like there's something missing in the PR title. Prevent what?

@lzap
Copy link
Member Author

lzap commented Jun 1, 2020

Ameded all your remarks. Also modified the workaround "HTTP Boot" so it actually works with bootdisk. Since network root prefix does not get carried over, this is needed to be used, either HTTP or HTTPS option must be selected manually or via parameter.

@ekohl I will update the title to match the changes

@lzap lzap changed the title Fixes #29952 - prevent when mac is present Fixes #29952 - make Grub2 template bootdisk-efi compatible Jun 1, 2020
proxy_http_port = @host.provision_interface.subnet.httpboot.setting(:HTTPBoot, "http_port")
proxy_https_port = @host.provision_interface.subnet.httpboot.setting(:HTTPBoot, "https_port")
# workaround around "no DNS server configured" in grub: https://bugzilla.redhat.com/show_bug.cgi?id=1842509
proxy_host = dns_lookup(@host.provision_interface.subnet.httpboot.hostname)
Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like grub is not able to initialize DNS or parsing of hostname with port (satellite:8000) does not resolve correctly. This hack does allow the workflow, without it it would not be possible. I suggest to go with this and we can remove it later once the BZ is fixed. https://bugzilla.redhat.com/show_bug.cgi?id=1842509

Comment on lines 35 to 37
proxy_http_port = @host.provision_interface.subnet.httpboot.setting(:HTTPBoot, "http_port")
proxy_https_port = @host.provision_interface.subnet.httpboot.setting(:HTTPBoot, "https_port")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm getting

undefined method '#setting' for SmartProxy::Jail (SmartProxy)

Is there a PR that exposes setting on smart proxy jail t hat I missed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Damn, I had to turn it off on my instance and forgot this one. Lemme update the other PR then.

@lzap lzap force-pushed the bootif-bootdisk-fix-29952 branch from 43d47e4 to f2f58d2 Compare June 2, 2020 10:58
@lzap
Copy link
Member Author

lzap commented Jun 2, 2020

Pushed, the absolute addressing must be kept because of another grub bug discovered moments ago, so I have added links to both bugs (one is fixed already, one is new - from today).

@lzap
Copy link
Member Author

lzap commented Jun 3, 2020

Last minor change which I missed since I was testing with an IP and not hostname:

diff --git a/app/views/foreman_bootdisk/generic_efi_host.erb b/app/views/foreman_bootdisk/generic_efi_host.erb
index 07f243a..b798d2e 100644
--- a/app/views/foreman_bootdisk/generic_efi_host.erb
+++ b/app/views/foreman_bootdisk/generic_efi_host.erb
@@ -16,6 +16,7 @@ echo " * HTTP ONLY (change the template for HTTPS)"
 echo " * ISC DHCP (other servers not tested)"
 echo " * GRUB FROM RHEL 8.3+/7.9+ (when generating the image)"
 echo " * EFI HTTP or HTTPS grub entry must be selected in menu"
+echo " * DNS must resolve proxy hostname via DNS proxy if set"
 echo "*******************************************************"
 sleep 5
 <%
@@ -26,8 +27,9 @@ proxy_proto = "http"
 @subnet.httpboot? || bootdisk_raise("Requires a proxy with httpboot feature")
 @subnet.template? || bootdisk_raise("Requires a proxy with template feature")
 proxy_port = @subnet.httpboot.setting(:HTTPBoot, "#{proxy_proto}_port") || bootdisk_raise("Requires proxy with #{proxy_proto}_port exposed setting")
-proxy_httpboot_host = @subnet.httpboot.hostname
-proxy_template_host = @subnet.template.hostname
+# Workaround for "no DNS server configured" https://bugzilla.redhat.com/show_bug.cgi?id=1842509
+proxy_httpboot_host = dns_lookup(@subnet.httpboot.hostname)
+proxy_template_host = dns_lookup(@subnet.template.hostname)
 -%>
 echo
 net_ls_cards

@lzap lzap force-pushed the bootif-bootdisk-fix-29952 branch from f2f58d2 to 05f8c58 Compare June 4, 2020 09:50
Copy link
Contributor

@adamruzicka adamruzicka left a comment

Choose a reason for hiding this comment

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

This works well

@adamruzicka adamruzicka merged commit 055f497 into theforeman:develop Jun 5, 2020
@adamruzicka
Copy link
Contributor

Thank you @lzap!

@lzap lzap deleted the bootif-bootdisk-fix-29952 branch June 5, 2020 07:53
lzap added a commit to lzap/community-templates that referenced this pull request Jun 5, 2020
@lzap
Copy link
Member Author

lzap commented Jun 5, 2020

mmoll pushed a commit that referenced this pull request Jun 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants