From cc7e3e92feac49112f6ab594895f1a6ad3f57eeb Mon Sep 17 00:00:00 2001 From: Yang JianFeng Date: Thu, 29 Nov 2018 20:18:31 +0800 Subject: [PATCH] Fix the bug about DHCP port whose network has multiple subnets. When a subnet's enable_dhcp attribute is updated, we must restart dhcp device. So,when we decide whether 'restart' or 'reload_allocations' in refresh_dhcp_helper function we only compare the cidr of subnets which enabled dhcp. The previous logic only calls 'restart' when deleting or adding a subnet. This may cause the dhcp port not updated when the subnet's enable_dhcp is updated to True. Change-Id: Ic547946ac786c5fab82b4ee7078bf86483f51eb5 Closes-Bug: #1805824 (cherry picked from commit 9aa7af82210f1ffb6274399648962b65162e17d1) --- neutron/agent/dhcp/agent.py | 6 ++-- neutron/tests/unit/agent/dhcp/test_agent.py | 39 ++++++++++++++++++++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/neutron/agent/dhcp/agent.py b/neutron/agent/dhcp/agent.py index 07621f4198e..27d417ae6f0 100644 --- a/neutron/agent/dhcp/agent.py +++ b/neutron/agent/dhcp/agent.py @@ -334,14 +334,12 @@ class DhcpAgent(manager.Manager): if not any(s for s in network.subnets if s.enable_dhcp): self.disable_dhcp_helper(network.id) return - # NOTE(kevinbenton): we don't exclude dhcp disabled subnets because - # they still change the indexes used for tags old_non_local_subnets = getattr(old_network, 'non_local_subnets', []) new_non_local_subnets = getattr(network, 'non_local_subnets', []) old_cidrs = [s.cidr for s in (old_network.subnets + - old_non_local_subnets)] + old_non_local_subnets) if s.enable_dhcp] new_cidrs = [s.cidr for s in (network.subnets + - new_non_local_subnets)] + new_non_local_subnets) if s.enable_dhcp] if old_cidrs == new_cidrs: self.call_driver('reload_allocations', network) self.cache.put(network) diff --git a/neutron/tests/unit/agent/dhcp/test_agent.py b/neutron/tests/unit/agent/dhcp/test_agent.py index 9752eec42e0..6a68d8a39cd 100644 --- a/neutron/tests/unit/agent/dhcp/test_agent.py +++ b/neutron/tests/unit/agent/dhcp/test_agent.py @@ -942,7 +942,9 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): self.assertTrue(log.called) self.assertTrue(self.dhcp.schedule_resync.called) - def test_subnet_create_restarts_with_dhcp_disabled(self): + def test_subnet_create_end(self): + # We should call reload_allocations when subnet's enable_dhcp + # attribute isn't True. payload = dict(subnet=dhcp.DictModel( dict(network_id=fake_network.id, enable_dhcp=False, cidr='99.99.99.0/24', ip_version=4))) @@ -953,6 +955,18 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): self.dhcp.subnet_create_end(None, payload) + self.cache.assert_has_calls([mock.call.put(new_net)]) + self.call_driver.assert_called_once_with('reload_allocations', new_net) + + # We should call restart when subnet's enable_dhcp attribute is True. + self.call_driver.reset_mock() + payload = dict(subnet=dhcp.DictModel( + dict(network_id=fake_network.id, enable_dhcp=True, + cidr='99.99.88.0/24', ip_version=const.IP_VERSION_4))) + new_net = copy.deepcopy(fake_network) + new_net.subnets.append(payload['subnet']) + self.plugin.get_network_info.return_value = new_net + self.dhcp.subnet_create_end(None, payload) self.cache.assert_has_calls([mock.call.put(new_net)]) self.call_driver.assert_called_once_with('restart', new_net) @@ -970,6 +984,29 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): self.assertEqual({p.id for p in fake_network.ports}, self.dhcp.dhcp_ready_ports) + def test_subnet_update_dhcp(self): + payload = dict(subnet=dict(network_id=fake_network.id)) + self.cache.get_network_by_id.return_value = fake_network + new_net = copy.deepcopy(fake_network) + new_subnet1 = copy.deepcopy(fake_subnet1) + new_subnet2 = copy.deepcopy(fake_subnet2) + new_subnet2.enable_dhcp = True + new_net.subnets = [new_subnet1, new_subnet2] + self.plugin.get_network_info.return_value = new_net + self.dhcp.subnet_update_end(None, payload) + self.call_driver.assert_called_once_with('restart', new_net) + + self.call_driver.reset_mock() + self.cache.get_network_by_id.return_value = new_net + new_net2 = copy.deepcopy(new_net) + new_subnet1 = copy.deepcopy(new_subnet1) + new_subnet1.enable_dhcp = False + new_subnet2 = copy.deepcopy(new_subnet2) + new_net2.subnets = [new_subnet1, new_subnet2] + self.plugin.get_network_info.return_value = new_net2 + self.dhcp.subnet_update_end(None, payload) + self.call_driver.assert_called_once_with('restart', new_net2) + def test_subnet_update_end_restart(self): new_state = dhcp.NetModel(dict(id=fake_network.id, tenant_id=fake_network.tenant_id,