openstack/neutron
Code Issues Proposed changes

restart dnsmasq when subnet cidr set changes

Browse Source 
fixes bug 1045828

The original bug occurred because the set of subnet cidrs changed
without a restart of dnsmasq.  As a result, dnsmasq would not respond
with offers for fixed_ips assigned in the unknown cidr ranges even when
the underlying host file had properly updated.

This patch addresses the bug by restarting dnsmasq when the set of
subnet cidrs changes.  When the set does not change, it will reload the
options and allocations.

Change-Id: I0e257208750703f4a32c51d1323786e76e676bb6
This commit is contained in:
Mark McClain
2012-09-05 00:52:21 -04:00
parent aaec26c7db
commit 6fa761fd14
4 changed files with 85 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
quantum/agent/dhcp_agent.py
View File

@@ -120,16 +120,22 @@ class DhcpAgent(object):
of the network.
"""
if not self.cache.get_network_by_id(network_id):
old_network = self.cache.get_network_by_id(network_id)
if not old_network:
# DHCP current not running for network.
self.enable_dhcp_helper(network_id)
return self.enable_dhcp_helper(network_id)
network = self.plugin_rpc.get_network_info(network_id)
for subnet in network.subnets:
if subnet.enable_dhcp:
old_cidrs = set(s.cidr for s in old_network.subnets if s.enable_dhcp)
new_cidrs = set(s.cidr for s in network.subnets if s.enable_dhcp)
if new_cidrs and old_cidrs == new_cidrs:
self.call_driver('reload_allocations', network)
self.cache.put(network)
elif new_cidrs:
self.call_driver('restart', network)
self.cache.put(network)
self.call_driver('enable', network)
break
else:
self.disable_dhcp_helper(network.id)

9
quantum/agent/linux/dhcp.py
View File

@@ -76,12 +76,12 @@ class DhcpBase(object):
"""Enables DHCP for this network."""
@abc.abstractmethod
def disable(self):
def disable(self, retain_port=False):
"""Disable dhcp for this network."""
def restart(self):
"""Restart the dhcp service for the network."""
self.disable()
self.disable(retain_port=True)
self.enable()
@abc.abstractproperty
@@ -108,12 +108,12 @@ class DhcpLocalProcess(DhcpBase):
interface_name = self.device_delegate.setup(self.network,
reuse_existing=True)
if self.active:
self.reload_allocations()
self.restart()
elif self._enable_dhcp():
self.interface_name = interface_name
self.spawn_process()
def disable(self):
def disable(self, retain_port=False):
"""Disable DHCP for this network by killing the local process."""
pid = self.pid
@@ -125,6 +125,7 @@ class DhcpLocalProcess(DhcpBase):
else:
utils.execute(cmd, self.root_helper)
if not retain_port:
self.device_delegate.destroy(self.network, self.interface_name)
elif pid:

47
quantum/tests/unit/test_dhcp_agent.py
View File

@@ -46,6 +46,10 @@ fake_subnet2 = FakeModel('dddddddd-dddd-dddd-dddddddddddd',
network_id='12345678-1234-5678-1234567890ab',
enable_dhcp=False)
fake_subnet3 = FakeModel('bbbbbbbb-1111-2222-bbbbbbbbbbbb',
network_id='12345678-1234-5678-1234567890ab',
cidr='192.168.1.1/24', enable_dhcp=True)
fake_fixed_ip = FakeModel('', subnet=fake_subnet1, ip_address='172.9.9.9')
fake_port1 = FakeModel('12345678-1234-aaaa-1234567890ab',
@@ -218,18 +222,21 @@ class TestDhcpAgentEventHandler(unittest.TestCase):
disable.assertCalledOnceWith(fake_network.id)
def test_refresh_dhcp_helper_no_dhcp_enabled_networks(self):
network = FakeModel('12345678-1234-5678-1234567890ab',
network = FakeModel('net-id',
tenant_id='aaaaaaaa-aaaa-aaaa-aaaaaaaaaaaa',
admin_state_up=True,
subnets=[],
ports=[])
self.cache.get_network_by_id.return_value = network
self.plugin.get_network_info.return_value = network
with mock.patch.object(self.dhcp, 'disable_dhcp_helper') as disable:
self.dhcp.refresh_dhcp_helper(network.id)
disable.called_once_with_args(network.id)
self.assertFalse(self.cache.called)
self.assertFalse(self.call_driver.called)
self.cache.assert_has_calls(
[mock.call.get_network_by_id('net-id')])
def test_subnet_update_end(self):
payload = dict(subnet=dict(network_id=fake_network.id))
@@ -239,17 +246,47 @@ class TestDhcpAgentEventHandler(unittest.TestCase):
self.dhcp.subnet_update_end(payload)
self.cache.assert_has_calls([mock.call.put(fake_network)])
self.call_driver.assert_called_once_with('enable', fake_network)
self.call_driver.assert_called_once_with('reload_allocations',
fake_network)
def test_subnet_update_end(self):
new_state = FakeModel(fake_network.id,
tenant_id=fake_network.tenant_id,
admin_state_up=True,
subnets=[fake_subnet1, fake_subnet3],
ports=[fake_port1])
payload = dict(subnet=dict(network_id=fake_network.id))
self.cache.get_network_by_id.return_value = fake_network
self.plugin.get_network_info.return_value = new_state
self.dhcp.subnet_update_end(payload)
self.cache.assert_has_calls([mock.call.put(new_state)])
self.call_driver.assert_called_once_with('restart',
new_state)
def test_subnet_update_end_delete_payload(self):
prev_state = FakeModel(fake_network.id,
tenant_id=fake_network.tenant_id,
admin_state_up=True,
subnets=[fake_subnet1, fake_subnet3],
ports=[fake_port1])
payload = dict(subnet_id=fake_subnet1.id)
self.cache.get_network_by_subnet_id.return_value = fake_network
self.cache.get_network_by_subnet_id.return_value = prev_state
self.cache.get_network_by_id.return_value = prev_state
self.plugin.get_network_info.return_value = fake_network
self.dhcp.subnet_delete_end(payload)
self.cache.assert_has_calls([mock.call.put(fake_network)])
self.call_driver.assert_called_once_with('enable', fake_network)
self.cache.assert_has_calls([
mock.call.get_network_by_subnet_id(
'bbbbbbbb-bbbb-bbbb-bbbbbbbbbbbb'),
mock.call.get_network_by_id('12345678-1234-5678-1234567890ab'),
mock.call.put(fake_network)])
self.call_driver.assert_called_once_with('restart',
fake_network)
def test_port_update_end(self):
payload = dict(port=vars(fake_port2))

28
quantum/tests/unit/test_linux_dhcp.py
View File

@@ -148,8 +148,8 @@ class TestDhcpBase(unittest.TestCase):
def enable(self):
self.called.append('enable')
def disable(self):
self.called.append('disable')
def disable(self, retain_port=False):
self.called.append('disable %s' % retain_port)
def reload_allocations(self):
pass
@@ -160,7 +160,7 @@ class TestDhcpBase(unittest.TestCase):
c = SubClass()
c.restart()
self.assertEquals(c.called, ['disable', 'enable'])
self.assertEquals(c.called, ['disable True', 'enable'])
class LocalChild(dhcp.DhcpLocalProcess):
@@ -173,6 +173,9 @@ class LocalChild(dhcp.DhcpLocalProcess):
def reload_allocations(self):
self.called.append('reload')
def restart(self):
self.called.append('restart')
def spawn_process(self):
self.called.append('spawn')
@@ -248,7 +251,7 @@ class TestDhcpLocalProcess(TestBase):
device_delegate=delegate)
lp.enable()
self.assertEqual(lp.called, ['reload'])
self.assertEqual(lp.called, ['restart'])
def test_enable(self):
delegate = mock.Mock(return_value='tap0')
@@ -297,6 +300,23 @@ class TestDhcpLocalProcess(TestBase):
msg = log.call_args[0][0]
self.assertIn('No DHCP', msg)
def test_disable_retain_port(self):
attrs_to_mock = dict([(a, mock.DEFAULT) for a in
['active', 'interface_name', 'pid']])
delegate = mock.Mock()
network = FakeDualNetwork()
with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks:
mocks['active'].__get__ = mock.Mock(return_value=True)
mocks['pid'].__get__ = mock.Mock(return_value=5)
mocks['interface_name'].__get__ = mock.Mock(return_value='tap0')
lp = LocalChild(self.conf, network, device_delegate=delegate,
namespace='qdhcp-ns')
lp.disable(retain_port=True)
self.assertFalse(delegate.called)
exp_args = ['ip', 'netns', 'exec', 'qdhcp-ns', 'kill', '-9', 5]
self.execute.assert_called_once_with(exp_args, root_helper='sudo')
def test_disable(self):
attrs_to_mock = dict([(a, mock.DEFAULT) for a in
['active', 'interface_name', 'pid']])