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.

Sign up for GitHub

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

Jump to bottom

Add support for infinite DHCP leases #1176

Closed
wants to merge 1 commit into from
Closed

Add support for infinite DHCP leases #1176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions 02_configure_host.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,3 +243,13 @@ if [ "$DISABLE_MULTICAST" == "true" ]; then
sudo ebtables -A OUTPUT --pkttype-type multicast -p ip6 --ip6-dst ${dst} -j DROP
done
fi

# Hack to force use of infinite leases in versions of libvirt that don't
# support setting the lease time in the network config.
# This should be removed when we have a new enough version of libvirt to
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add what that version number would be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll see if I can find it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is done.

# do it properly. I believe any version 6.3.0 or above should have the feature.
if [ ${INFINITE_LEASES:-0} -eq 1 ]
then
./infinite-leases.sh &
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder should we redirect io to somewhere, I think its holding ./02_configure_host.sh from finishing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been using this locally and it hasn't caused any problems. Are you seeing something different?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I think I was mistaken when I mentioned 02_configure_host.sh , what I'm seeing is that after the deployment infinite-leases.sh is still running. If for some reason the loop to edit the leases file is executed then the terminal where I had run the make command from gets the output
"Added infinite leases to /var/lib/libvirt/dnsmasq/ostestbm.hostsfile"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I guess I've never had the script trigger after make was complete. I could probably remove the log message entirely. It was mostly there to let me know the script was working, but it isn't particularly necessary.

echo "$!" > "${WORKING_DIR}/infinite-lease-pid"
fi
6 changes: 6 additions & 0 deletions host_cleanup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,9 @@ sudo ebtables --flush

# Kill any lingering proxy
sudo pkill -f oc.*proxy

# Kill the infinite lease script
if [ -s "${WORKING_DIR}/infinite-lease-pid" ]; then
kill $(cat "${WORKING_DIR}/infinite-lease-pid")
rm "${WORKING_DIR}/infinite-lease-pid"
fi
19 changes: 19 additions & 0 deletions infinite-leases.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#!/bin/bash

NETWORK_NAME=${CLUSTER_NAME}bm
FILENAME=/var/lib/libvirt/dnsmasq/${NETWORK_NAME}.hostsfile

# We need to keep running even after setting the infinite leases because
# they are overwritten multiple times in the deployment process.
while :
do
if grep -q -v infinite $FILENAME
then
sudo perl -pi -e 's/(.*?)(,infinite|)$/\1,infinite/' $FILENAME
Copy link
Collaborator

Choose a reason for hiding this comment

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

adding this to the hostsfile means that we cant restart the libvirt network (I guess the lease-init script can't parse it), maybe we don't care but I thought it was worth pointing out.

[derekh@u07 dev-scripts]$ sudo virsh net-start ostestbm
error: Failed to start network ostestbm
error: internal error: Child process (VIR_BRIDGE_NAME=ostestbm /usr/sbin/dnsmasq --conf-file=/var/lib/libvirt/dnsmasq/ostestbm.conf --leasefile-ro --dhcp-script=/usr/libexec/libvirt_leaseshelper) unexpected exit status 11: 
dnsmasq: lease-init script returned exit code 1
[derekh@u07 dev-scripts]$ kill: not enough arguments
Added infinite leases to /var/lib/libvirt/dnsmasq/ostestbm.hostsfile

Also the scripts that generates the clouds.yaml no longer works, as it can't get the host IP addresses with virsh

[derekh@u07 dev-scripts]$ virsh net-dhcp-leases ostestbm
error: failed to get network 'ostestbm'
error: Network not found: no network with matching name 'ostestbm'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. There was a bug opened for it: https://bugzilla.redhat.com/show_bug.cgi?id=1908053

It was just a problem with libvirt's handling of infinite leases in general. Even setting them properly through the libvirt xml was failing. I guess this will be fixed when we get to a newer version of libvirt, at which point we should be able to drop this hack.

It is a bit of an issue though since one of the reasons I wanted to get this in dev-scripts was so we could use it for ci, which won't work if the post-deploy steps are failing. Still useful for development, but if we do get a ci job configured before getting a fixed libvirt I'll have to find some workaround.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess if its not on by default then its not a big problem, and it'll have to be sorted somehow before getting switched on.


pid=$(ps aux | grep dnsmasq | grep "$NETWORK_NAME" | grep -v root | awk '{print $2}')
sudo kill -s SIGHUP $pid
echo "Added infinite leases to $FILENAME"
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is triggered after make completes, as is it is output to where ever make was run from

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't really designed to be run after deployment. The problem the script was designed to solve is the fact that the hosts file gets rewritten multiple times during the deployment, so if you manually added the leases they might get removed again. After deployment that's no longer a problem.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So would it be ok to just kill the script at the end of the deployment?

fi
sleep 10
done