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 9aa7af8221
)
This commit is contained in:
parent
ba3f2839e9
commit
ce2ddcbf3f
|
@ -336,14 +336,12 @@ class DhcpAgent(manager.Manager):
|
||||||
if not any(s for s in network.subnets if s.enable_dhcp):
|
if not any(s for s in network.subnets if s.enable_dhcp):
|
||||||
self.disable_dhcp_helper(network.id)
|
self.disable_dhcp_helper(network.id)
|
||||||
return
|
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', [])
|
old_non_local_subnets = getattr(old_network, 'non_local_subnets', [])
|
||||||
new_non_local_subnets = getattr(network, 'non_local_subnets', [])
|
new_non_local_subnets = getattr(network, 'non_local_subnets', [])
|
||||||
old_cidrs = [s.cidr for s in (old_network.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_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:
|
if old_cidrs == new_cidrs:
|
||||||
self.call_driver('reload_allocations', network)
|
self.call_driver('reload_allocations', network)
|
||||||
self.cache.put(network)
|
self.cache.put(network)
|
||||||
|
|
|
@ -942,7 +942,9 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
|
||||||
self.assertTrue(log.called)
|
self.assertTrue(log.called)
|
||||||
self.assertTrue(self.dhcp.schedule_resync.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(
|
payload = dict(subnet=dhcp.DictModel(
|
||||||
dict(network_id=fake_network.id, enable_dhcp=False,
|
dict(network_id=fake_network.id, enable_dhcp=False,
|
||||||
cidr='99.99.99.0/24', ip_version=4)))
|
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.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.cache.assert_has_calls([mock.call.put(new_net)])
|
||||||
self.call_driver.assert_called_once_with('restart', 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.assertEqual({p.id for p in fake_network.ports},
|
||||||
self.dhcp.dhcp_ready_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):
|
def test_subnet_update_end_restart(self):
|
||||||
new_state = dhcp.NetModel(dict(id=fake_network.id,
|
new_state = dhcp.NetModel(dict(id=fake_network.id,
|
||||||
tenant_id=fake_network.tenant_id,
|
tenant_id=fake_network.tenant_id,
|
||||||
|
|
Loading…
Reference in New Issue