diff --git a/quantum/agent/dhcp_agent.py b/quantum/agent/dhcp_agent.py index 2b968fcd93..9196a2fbef 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 e6dcc25cec..31146c0853 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 e1b5bce702..fe71ec5187 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 6804bbf48d..8a32f81c77 100644 --- a/quantum/tests/unit/test_linux_dhcp.py +++ b/quantum/tests/unit/test_linux_dhcp.py @@ -164,8 +164,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 @@ -176,7 +176,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): @@ -189,6 +189,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') @@ -264,7 +267,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') @@ -313,6 +316,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']])