Persist DHCP leases to a local database
Due to issues caused by dnsmasq restarts sending DHCPNAKs,
change Ieff0236670c1403b5d79ad8e50d7574c1b694e34 passed the
'dhcp-authoritative' option to dnsmasq. While this solved the
restart issue, it broke the multi-DHCP server scenario because
the dnsmasq instances will NAK requests to a server ID that
isn't their own.
Problem DHCP Request Lifecycle:
Client: DHCPDISCOVER(broadcast)
Server1: DHCPOFFER
Server2: DHCPOFFER
Client: DHCPREQUEST(broadcast with Server-ID=Server1)
Server1: DHCPACK
Server2: DHCPNAK(in response to observed DHCPREQUEST with other Server-ID)
^---Causes issues
This change removes the authoritative option so NAKs are not
send in response to DHCPREQUEST's to other servers. To handle
the original issue that Ieff0236670c1403b5d79ad8e50d7574c1b694e34
was inteded to address, this patch also allows changes to be persisted
to a local lease file.
In order to handle the issue where a DHCP server may be scheduled
to another agent, a fake lease file is generated for dnsmasq to start
with. The contents are populated based on all of the known ports for
a network. This should prevent dnsmasq from NAKing clients renewing
leases issued before it was restarted/rescheduled.
Closes-Bug: #1457900
Change-Id: Idc91602bf8c474467e596cbd5cbaa8898952c841
(cherry picked from commit 98d8ad911d)
This commit is contained in:
Kevin Benton
parent
e459d2d3ca
commit
c07687c985
|
|
@ -18,6 +18,7 @@ import collections
|
|||
import os
|
||||
import re
|
||||
import shutil
|
||||
import time
|
||||
|
||||
import netaddr
|
||||
from oslo_config import cfg
|
||||
|
|
@ -310,8 +311,7 @@ class Dnsmasq(DhcpLocalProcess):
|
|||
'--dhcp-hostsfile=%s' % self.get_conf_file_name('host'),
|
||||
'--addn-hosts=%s' % self.get_conf_file_name('addn_hosts'),
|
||||
'--dhcp-optsfile=%s' % self.get_conf_file_name('opts'),
|
||||
'--leasefile-ro',
|
||||
'--dhcp-authoritative',
|
||||
'--dhcp-leasefile=%s' % self.get_conf_file_name('leases'),
|
||||
]
|
||||
|
||||
possible_leases = 0
|
||||
|
|
@ -379,6 +379,9 @@ class Dnsmasq(DhcpLocalProcess):
|
|||
|
||||
def spawn_process(self):
|
||||
"""Spawn the process, if it's not spawned already."""
|
||||
# we only need to generate the lease file the first time dnsmasq starts
|
||||
# rather than on every reload since dnsmasq will keep the file current
|
||||
self._output_init_lease_file()
|
||||
self._spawn_or_reload_process(reload_with_HUP=False)
|
||||
|
||||
def _spawn_or_reload_process(self, reload_with_HUP):
|
||||
|
|
@ -461,6 +464,58 @@ class Dnsmasq(DhcpLocalProcess):
|
|||
fqdn = '%s.%s' % (fqdn, self.conf.dhcp_domain)
|
||||
yield (port, alloc, hostname, fqdn)
|
||||
|
||||
def _output_init_lease_file(self):
|
||||
"""Write a fake lease file to bootstrap dnsmasq.
|
||||
|
||||
The generated file is passed to the --dhcp-leasefile option of dnsmasq.
|
||||
This is used as a bootstrapping mechanism to avoid NAKing active leases
|
||||
when a dhcp server is scheduled to another agent. Using a leasefile
|
||||
will also prevent dnsmasq from NAKing or ignoring renewals after a
|
||||
restart.
|
||||
|
||||
Format is as follows:
|
||||
epoch-timestamp mac_addr ip_addr hostname client-ID
|
||||
"""
|
||||
filename = self.get_conf_file_name('leases')
|
||||
buf = six.StringIO()
|
||||
|
||||
LOG.debug('Building initial lease file: %s', filename)
|
||||
# we make up a lease time for the database entry
|
||||
if self.conf.dhcp_lease_duration == -1:
|
||||
# Even with an infinite lease, a client may choose to renew a
|
||||
# previous lease on reboot or interface bounce so we should have
|
||||
# an entry for it.
|
||||
# Dnsmasq timestamp format for an infinite lease is is 0.
|
||||
timestamp = 0
|
||||
else:
|
||||
timestamp = int(time.time()) + self.conf.dhcp_lease_duration
|
||||
dhcp_enabled_subnet_ids = [s.id for s in self.network.subnets
|
||||
if s.enable_dhcp]
|
||||
for (port, alloc, hostname, name) in self._iter_hosts():
|
||||
# don't write ip address which belongs to a dhcp disabled subnet.
|
||||
if not alloc or alloc.subnet_id not in dhcp_enabled_subnet_ids:
|
||||
continue
|
||||
|
||||
ip_address = self._format_address_for_dnsmasq(alloc.ip_address)
|
||||
# all that matters is the mac address and IP. the hostname and
|
||||
# client ID will be overwritten on the next renewal.
|
||||
buf.write('%s %s %s * *\n' %
|
||||
(timestamp, port.mac_address, ip_address))
|
||||
contents = buf.getvalue()
|
||||
utils.replace_file(filename, contents)
|
||||
LOG.debug('Done building initial lease file %s with contents:\n%s',
|
||||
filename, contents)
|
||||
return filename
|
||||
|
||||
@staticmethod
|
||||
def _format_address_for_dnsmasq(address):
|
||||
# (dzyu) Check if it is legal ipv6 address, if so, need wrap
|
||||
# it with '[]' to let dnsmasq to distinguish MAC address from
|
||||
# IPv6 address.
|
||||
if netaddr.valid_ipv6(address):
|
||||
return '[%s]' % address
|
||||
return address
|
||||
|
||||
def _output_hosts_file(self):
|
||||
"""Writes a dnsmasq compatible dhcp hosts file.
|
||||
|
||||
|
|
@ -496,12 +551,7 @@ class Dnsmasq(DhcpLocalProcess):
|
|||
if alloc.subnet_id not in dhcp_enabled_subnet_ids:
|
||||
continue
|
||||
|
||||
# (dzyu) Check if it is legal ipv6 address, if so, need wrap
|
||||
# it with '[]' to let dnsmasq to distinguish MAC address from
|
||||
# IPv6 address.
|
||||
ip_address = alloc.ip_address
|
||||
if netaddr.valid_ipv6(ip_address):
|
||||
ip_address = '[%s]' % ip_address
|
||||
ip_address = self._format_address_for_dnsmasq(alloc.ip_address)
|
||||
|
||||
if getattr(port, 'extra_dhcp_opts', False):
|
||||
buf.write('%s,%s,%s,%s%s\n' %
|
||||
|
|
|
|||
|
|
@ -833,8 +833,7 @@ class TestDnsmasq(TestBase):
|
|||
'--dhcp-hostsfile=/dhcp/%s/host' % network.id,
|
||||
'--addn-hosts=/dhcp/%s/addn_hosts' % network.id,
|
||||
'--dhcp-optsfile=/dhcp/%s/opts' % network.id,
|
||||
'--leasefile-ro',
|
||||
'--dhcp-authoritative']
|
||||
'--dhcp-leasefile=/dhcp/%s/leases' % network.id]
|
||||
|
||||
seconds = ''
|
||||
if lease_duration == -1:
|
||||
|
|
@ -956,6 +955,34 @@ class TestDnsmasq(TestBase):
|
|||
self._test_spawn(['--conf-file=', '--domain=openstacklocal'],
|
||||
network)
|
||||
|
||||
def _test_output_init_lease_file(self, timestamp):
|
||||
expected = [
|
||||
'00:00:80:aa:bb:cc 192.168.0.2 * *',
|
||||
'00:00:f3:aa:bb:cc [fdca:3ba5:a17a:4ba3::2] * *',
|
||||
'00:00:0f:aa:bb:cc 192.168.0.3 * *',
|
||||
'00:00:0f:aa:bb:cc [fdca:3ba5:a17a:4ba3::3] * *',
|
||||
'00:00:0f:rr:rr:rr 192.168.0.1 * *\n']
|
||||
expected = "\n".join(['%s %s' % (timestamp, l) for l in expected])
|
||||
with mock.patch.object(dhcp.Dnsmasq, 'get_conf_file_name') as conf_fn:
|
||||
conf_fn.return_value = '/foo/leases'
|
||||
dm = self._get_dnsmasq(FakeDualNetwork())
|
||||
dm._output_init_lease_file()
|
||||
self.safe.assert_called_once_with('/foo/leases', expected)
|
||||
|
||||
@mock.patch('time.time')
|
||||
def test_output_init_lease_file(self, tmock):
|
||||
self.conf.set_override('dhcp_lease_duration', 500)
|
||||
tmock.return_value = 1000000
|
||||
# lease duration should be added to current time
|
||||
timestamp = 1000000 + 500
|
||||
self._test_output_init_lease_file(timestamp)
|
||||
|
||||
def test_output_init_lease_file_infinite_duration(self):
|
||||
self.conf.set_override('dhcp_lease_duration', -1)
|
||||
# when duration is infinite, lease db timestamp should be 0
|
||||
timestamp = 0
|
||||
self._test_output_init_lease_file(timestamp)
|
||||
|
||||
def _test_output_opts_file(self, expected, network, ipm_retval=None):
|
||||
with mock.patch.object(dhcp.Dnsmasq, 'get_conf_file_name') as conf_fn:
|
||||
conf_fn.return_value = '/foo/opts'
|
||||
|
|
|
|||
Loading…
Reference in New Issue