Skip to content
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.

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

Add support for UEFI HTTP Boot #210

Closed
Tracked by #190
rgl opened this issue Oct 6, 2021 · 4 comments · Fixed by #216 or #228
Closed
Tracked by #190

Add support for UEFI HTTP Boot #210

rgl opened this issue Oct 6, 2021 · 4 comments · Fixed by #216 or #228
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@rgl
Copy link
Contributor

rgl commented Oct 6, 2021

Expected Behaviour

I expected to be able to use UEFI HTTP Boot to boot a machine.

Current Behaviour

Only PXE Boot is supported.

Possible Solution

I've got dnsmasq serving HTTP Boot at:

https://github.com/rgl/talos-vagrant/blob/6ce54f83e4066bc60415d697f5c4a7a6cd5ac561/provision-dnsmasq.sh#L53-L59

@mmlb
Copy link
Contributor

mmlb commented Oct 6, 2021

Am I right thinking that you want to keep boot's dhcp support and detect a UEFI machine and serve the http script instead of iPXE binary?

@rgl
Copy link
Contributor Author

rgl commented Oct 6, 2021

Mainly replace PXE TFTP with UEFI HTTP and use it to serve ipxe.efi. All the rest stays the same.

@mmlb
Copy link
Contributor

mmlb commented Oct 6, 2021

ahh that sounds like a really nice feature indeed!

@mmlb mmlb added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 6, 2021
@rgl rgl mentioned this issue Oct 9, 2021
38 tasks
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Oct 9, 2021
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Oct 9, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Oct 9, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Oct 9, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
@rgl rgl mentioned this issue Oct 9, 2021
3 tasks
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Oct 10, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Oct 10, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Oct 16, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Oct 17, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Oct 17, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Oct 17, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Nov 4, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Nov 4, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Nov 5, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Nov 5, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
rgl added a commit to rgl/tinkerbell-boots that referenced this issue Nov 5, 2021
this closes tinkerbell#210

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
@mmlb
Copy link
Contributor

mmlb commented Dec 2, 2021

Re opening because I had to revert #216 (due to #225). I've split the one commit from this PR to multiple independent ones to facilitate bisecting (currently at https://github.com/mmlb/boots/tree/split-and-fix-http-boot-for-c2.large.arm for now).

@mmlb mmlb reopened this Dec 2, 2021
@mergify mergify bot closed this as completed in #228 Dec 9, 2021
mergify bot added a commit that referenced this issue Dec 9, 2021
## Description

This is a revert of the revert that occurred in #217 + fix that caused the original revert.
There's also removal of what is effectively useless/dead code (isPXEClient/pxeClient var/handling).
I also removed the leading `/` from `/nonexistent` because a `/` is already prefixed and `//nonexistent` has bothered me for years.

## Why is this needed

#216 was a great contribution from @rgl, unfortunately we ran into issues on some of our Ampere eMAG systems (#225).
Turns out that the eMAG boot firmware just straight up ignores filename values if the response packet is marked as `PXEClient`.
#216 changed the logic subtly by setting the response packet's class as `HTTPClient` if http was supported by the booting machine, and otherwise setting `PXEClient`. Before #216 we would set `PXEClient` only for the dhcp responses that were going out to our tftp'd build of iPXE, the initial filename: response was `PXEClient`-less.

Once I put back the only `PXEClient` for our iPXE everything worked out fine again (tested using sandbox), but then I realized that now pxeClient was likely useless because I suspected iPXE did not care/require it. I tested this too and found that iPXE is happy to tftp a file provided by a `PXEClient`-less DHCP response. So why keep code around that is effectively useless? Less code is better than any code so that caused me to write up d1878bd.

Fixes #210
Fixes #225

This debugging also opens up the way to bring back #149 as that had to be reverted due to the same "always set `PXEClient`" issue that I was not equipped to debug easily/quickly then.

## How Has This Been Tested?

Lots of testing on EM machines using github.com/tinkerbell/sandbox's terraform support (I plan to clean up and PR https://github.com/mmlb/sandbox/tree/fix-terraform-and-add-arm64-support at some point).

## How are existing users impacted? What migration steps/scripts do we need?

Back to faster/better iPXE binary downloads by utilizing TCP/HTTP instead of UDP/TFTP.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
2 participants