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

Implemented support for proxydhcp variables (disbaled by default) #953

Merged
merged 12 commits into from
Oct 24, 2021
Merged

Implemented support for proxydhcp variables (disbaled by default) #953

merged 12 commits into from
Oct 24, 2021

Conversation

notepass
Copy link
Contributor

@notepass notepass commented Aug 5, 2021

I've implemented the feature requested in #952
I'm not entirely sure if the type of implementation matches what the project wants. By default netboot.xyz will behave the same as before. Only after a change to "proxy-dhcp-vars.ipxe" is made, the new logic will be enabled.
The overall logic is as follows:

  • Check if a proxydhcp is set
    • If no
      • Set the "next-server" provided by the normal DHCP as tftp server
    • If yes
      • Request proxy-dhcp-vars.ipxe from the next-server defined by the normal DHCP to attempt automatic conflict resolution
      • Check the content of the variable "force_use_root_dhcp_settings"
        • If it is true: Use the next-server provided by the normal DHCP server as tftp server
        • If it is false: Use the next-server provided by the proxy DHCP server as tftp server
        • If it is not set or another value: Prompt the user to press F5 to accept the settings from the proxy server. If no key is pressed in 4 seconds, the server provided by the normal DHCP will be used

This logic could be simplified by removing the additional config file, but than there could be changes for the end user (e.g. the newly added prompt), or the proxydhcp could be used whenever it is detected. The code can be changed to do either of this, I'm completely open for feedback on this.

Edit: The changes currently only impact UEFI booting, as BIOS booting doesn't seem to have access to proxydhcp info

Copy link
Member

@antonym antonym left a comment

Choose a reason for hiding this comment

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

Requested a few changes but I think the general idea makes sense.

roles/netbootxyz/templates/menu/proxy-dhcp-vars.ipxe Outdated Show resolved Hide resolved
roles/netbootxyz/templates/disks/netboot.xyz.j2 Outdated Show resolved Hide resolved
roles/netbootxyz/templates/disks/netboot.xyz.j2 Outdated Show resolved Hide resolved
roles/netbootxyz/templates/disks/netboot.xyz.j2 Outdated Show resolved Hide resolved
roles/netbootxyz/templates/disks/netboot.xyz.j2 Outdated Show resolved Hide resolved
roles/netbootxyz/templates/local-vars.ipxe.j2 Outdated Show resolved Hide resolved
roles/netbootxyz/templates/disks/netboot.xyz.j2 Outdated Show resolved Hide resolved
roles/netbootxyz/templates/menu/netinfo.ipxe.j2 Outdated Show resolved Hide resolved
roles/netbootxyz/templates/menu/netinfo.ipxe.j2 Outdated Show resolved Hide resolved
@notepass notepass requested a review from antonym August 20, 2021 19:17
roles/netbootxyz/templates/disks/netboot.xyz.j2 Outdated Show resolved Hide resolved
roles/netbootxyz/templates/disks/netboot.xyz.j2 Outdated Show resolved Hide resolved
roles/netbootxyz/templates/menu/netinfo.ipxe.j2 Outdated Show resolved Hide resolved
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@notepass
Copy link
Contributor Author

Sorry for the late reply, I was tageled up in tape drive stuff (heh).
I've implemented the requested changes

@antonym antonym self-assigned this Oct 24, 2021
@antonym
Copy link
Member

antonym commented Oct 24, 2021

Have finally got around to setting up an environment to test with proxy working, so merging this in as is to build some images with it, test, and tune. I think the menu vars need to be reworked a bit as proxydhcp isn't a var thats set that's set but overall looks good. This should at least get us some builds to test and iterate on.

@antonym antonym merged commit 523a6f8 into netbootxyz:development Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants