From: Mark McClain Date: Wed, 5 Sep 2012 04:52:21 +0000 (-0400) Subject: restart dnsmasq when subnet cidr set changes X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=6fa761fd14f634d4194eaa8cd64dbfc1821aac30;p=openstack-build%2Fneutron-build.git restart dnsmasq when subnet cidr set changes 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 --- diff --git a/quantum/agent/dhcp_agent.py b/quantum/agent/dhcp_agent.py index 2b968fcd9..9196a2fbe 100644 --- a/quantum/agent/dhcp_agent.py +++ b/quantum/agent/dhcp_agent.py @@ -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: - self.cache.put(network) - self.call_driver('enable', network) - break + + 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) else: self.disable_dhcp_helper(network.id) diff --git a/quantum/agent/linux/dhcp.py b/quantum/agent/linux/dhcp.py index add556b0a..ed875adbd 100644 --- a/quantum/agent/linux/dhcp.py +++ b/quantum/agent/linux/dhcp.py @@ -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,7 +125,8 @@ class DhcpLocalProcess(DhcpBase): else: utils.execute(cmd, self.root_helper) - self.device_delegate.destroy(self.network, self.interface_name) + if not retain_port: + self.device_delegate.destroy(self.network, self.interface_name) elif pid: LOG.debug(_('DHCP for %s pid %d is stale, ignoring command') % diff --git a/quantum/tests/unit/test_dhcp_agent.py b/quantum/tests/unit/test_dhcp_agent.py index e1b5bce70..fe71ec518 100644 --- a/quantum/tests/unit/test_dhcp_agent.py +++ b/quantum/tests/unit/test_dhcp_agent.py @@ -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)) diff --git a/quantum/tests/unit/test_linux_dhcp.py b/quantum/tests/unit/test_linux_dhcp.py index 93f1ae1f3..9ea831713 100644 --- a/quantum/tests/unit/test_linux_dhcp.py +++ b/quantum/tests/unit/test_linux_dhcp.py @@ -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']])