Block port update from unbound DHCP agent
Current DHCP port management in Neutron makes the server to clear the device_id while the agent is responsible for setting it. This may cause a potential race condition, for example during network rescheduling. The server aims to clear the device_id on a DHCP port and assign the network to another agent while the old agent might just be taking possession of the port. If the DHCP agent takes possession of the port (i.e., update port...set the device_id) before the server clears it, then there is no issue. However, if this happens after the clear operation by server then the DHCP port would be updated/marked to be owned by the old agent. When the new agent takes over the network scheduled to it, it won't be able to find a port to reuse so that an extra port might need to be created. This leads to two issues: 1) an extra port is created and never deleted; 2) the extra port creation may fail if there are no available IP addresses. This patch proposes a validation check to prevent an agent from updating a DHCP port unless the network is bound to that agent. Co-authored-by: Allain Legacy <Allain.legacy@windriver.com> Conflicts: neutron/api/rpc/handlers/dhcp_rpc.py Note(elod.illes): Conflict caused by missing patch (that consumes constants from neutron_lib), which should not be backported: Ie4bcffccf626a6e1de84af01f3487feb825f8b65 Closes-Bug: #1795126 Story: 2003919 Change-Id: Ie619516c07fb3dc9d025f64c0e1e59d5d808cb6f (cherry picked from commit b70ee4df885d8ca18cb3dbfbec9691ecc0321f09) (cherry picked from commit b9f9c021c9692855cae30b6be1481058b3474112)
This commit is contained in:
parent
4bf6cff372
commit
b2418bc248
@ -272,6 +272,13 @@ class DhcpRpcCallback(object):
|
||||
plugin = directory.get_plugin()
|
||||
return self._port_action(plugin, context, port, 'create_port')
|
||||
|
||||
def _is_dhcp_agent_hosting_network(self, plugin, context, host,
|
||||
network_id):
|
||||
"""Check whether a DHCP agent (host) is hosting a network."""
|
||||
agents = plugin.get_dhcp_agents_hosting_networks(context, [network_id],
|
||||
hosts=[host])
|
||||
return len(agents) != 0
|
||||
|
||||
@oslo_messaging.expected_exceptions(exceptions.IpAddressGenerationFailure)
|
||||
@db_api.retry_db_errors
|
||||
def update_dhcp_port(self, context, **kwargs):
|
||||
@ -282,11 +289,13 @@ class DhcpRpcCallback(object):
|
||||
port['port'][portbindings.HOST_ID] = host
|
||||
plugin = directory.get_plugin()
|
||||
try:
|
||||
network_id = port['port']['network_id']
|
||||
old_port = plugin.get_port(context, port['id'])
|
||||
if (old_port['device_id'] != n_const.DEVICE_ID_RESERVED_DHCP_PORT
|
||||
and old_port['device_id'] !=
|
||||
utils.get_dhcp_agent_device_id(port['port']['network_id'],
|
||||
host)):
|
||||
utils.get_dhcp_agent_device_id(network_id, host) or
|
||||
not self._is_dhcp_agent_hosting_network(plugin, context, host,
|
||||
network_id)):
|
||||
raise n_exc.DhcpPortInUse(port_id=port['id'])
|
||||
LOG.debug('Update dhcp port %(port)s '
|
||||
'from %(host)s.',
|
||||
|
@ -44,6 +44,10 @@ class TestDhcpRpcCallback(base.BaseTestCase):
|
||||
self.mock_set_dirty = set_dirty_p.start()
|
||||
self.utils_p = mock.patch('neutron.plugins.common.utils.create_port')
|
||||
self.utils = self.utils_p.start()
|
||||
self.agent_hosting_network_p = mock.patch.object(self.callbacks,
|
||||
'_is_dhcp_agent_hosting_network')
|
||||
self.mock_agent_hosting_network = self.agent_hosting_network_p.start()
|
||||
self.mock_agent_hosting_network.return_value = True
|
||||
self.segment_plugin = mock.MagicMock()
|
||||
directory.add_plugin('segments', self.segment_plugin)
|
||||
|
||||
@ -329,6 +333,38 @@ class TestDhcpRpcCallback(base.BaseTestCase):
|
||||
self.plugin.assert_has_calls([
|
||||
mock.call.update_port(mock.ANY, 'foo_port_id', expected_port)])
|
||||
|
||||
def test_update_dhcp_port_with_agent_not_hosting_network(self):
|
||||
port = {'port': {'network_id': 'foo_network_id',
|
||||
'device_owner': constants.DEVICE_OWNER_DHCP,
|
||||
'fixed_ips': [{'subnet_id': 'foo_subnet_id'}]}
|
||||
}
|
||||
self.plugin.get_port.return_value = {
|
||||
'device_id': constants.DEVICE_ID_RESERVED_DHCP_PORT}
|
||||
self.mock_agent_hosting_network.return_value = False
|
||||
self.assertRaises(exceptions.DhcpPortInUse,
|
||||
self.callbacks.update_dhcp_port,
|
||||
mock.Mock(),
|
||||
host='foo_host',
|
||||
port_id='foo_port_id',
|
||||
port=port)
|
||||
|
||||
def test__is_dhcp_agent_hosting_network(self):
|
||||
self.agent_hosting_network_p.stop()
|
||||
agent = mock.Mock()
|
||||
with mock.patch.object(self.plugin, 'get_dhcp_agents_hosting_networks',
|
||||
return_value=[agent]):
|
||||
ret = self.callbacks._is_dhcp_agent_hosting_network(self.plugin,
|
||||
mock.Mock(), host='foo_host', network_id='foo_network_id')
|
||||
self.assertTrue(ret)
|
||||
|
||||
def test__is_dhcp_agent_hosting_network_false(self):
|
||||
self.agent_hosting_network_p.stop()
|
||||
with mock.patch.object(self.plugin, 'get_dhcp_agents_hosting_networks',
|
||||
return_value=[]):
|
||||
ret = self.callbacks._is_dhcp_agent_hosting_network(self.plugin,
|
||||
mock.Mock(), host='foo_host', network_id='foo_network_id')
|
||||
self.assertFalse(ret)
|
||||
|
||||
def test_release_dhcp_port(self):
|
||||
port_retval = dict(id='port_id', fixed_ips=[dict(subnet_id='a')])
|
||||
self.plugin.get_ports.return_value = [port_retval]
|
||||
|
Loading…
x
Reference in New Issue
Block a user