FOGProject / fogproject

An open source computer cloning & management system
https://fogproject.org
GNU General Public License v3.0
1.09k stars 221 forks source link

Closes #510 #522

Closed lukebarone closed 1 year ago

lukebarone commented 1 year ago

Install log indicates that isc-dhcp-server is NO LONGER being installed. The configuration step is also marked as "Skipped".

Sebastian-Roth commented 1 year ago

@lukebarone Thanks for the PR on this!

While I really like your way of using the bash variable substitution I am wondering if it's wise to add dhcpd as you did. It might not be obvious but there is a difference between the variables dhcpd and dhcpname. The first is used to control the system service using systemctl commands while the later one is meant to hold the actual package name. So from my point of view the more logic fix would be to add [[ -z $dhcpname ]] && dhcpname="isc-dhcp-server" to ./lib/ubuntu/config.sh.

Beside that I still vote for using the bash variable substitution instead of the for loop.

And it would actually make sense to change the same loop code just a few lines before that as well.

If you are fine with that you can just push to your repos branch to update this PR.

lukebarone commented 1 year ago

Running through a test on a VM...

Works for me