Optimize the code that fixes the race condition of DHCP agent.
https://review.opendev.org/#/c/236983/
https://review.opendev.org/#/c/606383/
The above patchs that resolve the race condition of DHCP agent will
result in neutron-server raise DhcpPortInUse ERROR log. And, the
second patch may result in old dhcp agent create a redundant port.
Closes-Bug: #1829332
Change-Id: If7a7ac2f88ce5b0e799c1104c936735a6cc860aa
(cherry picked from commit 494b65d951
)
This commit is contained in:
parent
489a841bc5
commit
05804a325e
|
@ -177,7 +177,8 @@ class DhcpAgent(manager.Manager):
|
|||
if (isinstance(e, oslo_messaging.RemoteError) and
|
||||
e.exc_type == 'NetworkNotFound' or
|
||||
isinstance(e, exceptions.NetworkNotFound)):
|
||||
LOG.debug("Network %s has been deleted.", network.id)
|
||||
LOG.debug("Network %s has been removed from the agent "
|
||||
"or deleted from DB.", network.id)
|
||||
else:
|
||||
LOG.exception('Unable to %(action)s dhcp for %(net_id)s.',
|
||||
{'net_id': network.id, 'action': action})
|
||||
|
|
|
@ -26,7 +26,6 @@ from neutron_lib import constants
|
|||
from neutron_lib import exceptions
|
||||
from neutron_lib.utils import file as file_utils
|
||||
from oslo_log import log as logging
|
||||
import oslo_messaging
|
||||
from oslo_utils import excutils
|
||||
from oslo_utils import fileutils
|
||||
from oslo_utils import uuidutils
|
||||
|
@ -1369,16 +1368,9 @@ class DeviceManager(object):
|
|||
for port in network.ports:
|
||||
port_device_id = getattr(port, 'device_id', None)
|
||||
if port_device_id == constants.DEVICE_ID_RESERVED_DHCP_PORT:
|
||||
try:
|
||||
port = self.plugin.update_dhcp_port(
|
||||
port.id, {'port': {'network_id': network.id,
|
||||
'device_id': device_id}})
|
||||
except oslo_messaging.RemoteError as e:
|
||||
if e.exc_type == 'DhcpPortInUse':
|
||||
LOG.info("Skipping DHCP port %s as it is "
|
||||
"already in use", port.id)
|
||||
continue
|
||||
raise
|
||||
port = self.plugin.update_dhcp_port(
|
||||
port.id, {'port': {'network_id': network.id,
|
||||
'device_id': device_id}})
|
||||
if port:
|
||||
return port
|
||||
|
||||
|
|
|
@ -279,6 +279,7 @@ class DhcpRpcCallback(object):
|
|||
hosts=[host])
|
||||
return len(agents) != 0
|
||||
|
||||
@oslo_messaging.expected_exceptions(exceptions.NetworkNotFound)
|
||||
@oslo_messaging.expected_exceptions(exceptions.IpAddressGenerationFailure)
|
||||
@db_api.retry_db_errors
|
||||
def update_dhcp_port(self, context, **kwargs):
|
||||
|
@ -294,10 +295,14 @@ class DhcpRpcCallback(object):
|
|||
if (old_port['device_id'] !=
|
||||
constants.DEVICE_ID_RESERVED_DHCP_PORT and
|
||||
old_port['device_id'] !=
|
||||
utils.get_dhcp_agent_device_id(network_id, host) or
|
||||
not self._is_dhcp_agent_hosting_network(plugin, context, host,
|
||||
network_id)):
|
||||
raise exceptions.DhcpPortInUse(port_id=port['id'])
|
||||
utils.get_dhcp_agent_device_id(network_id, host)):
|
||||
return
|
||||
if not self._is_dhcp_agent_hosting_network(plugin, context, host,
|
||||
network_id):
|
||||
LOG.warning("The DHCP agent on %(host)s does not host the "
|
||||
"network %(net_id)s.", {"host": host,
|
||||
"net_id": network_id})
|
||||
raise exceptions.NetworkNotFound(net_id=network_id)
|
||||
LOG.debug('Update dhcp port %(port)s '
|
||||
'from %(host)s.',
|
||||
{'port': port,
|
||||
|
@ -307,7 +312,6 @@ class DhcpRpcCallback(object):
|
|||
LOG.debug('Host %(host)s tried to update port '
|
||||
'%(port_id)s which no longer exists.',
|
||||
{'host': host, 'port_id': port['id']})
|
||||
return None
|
||||
|
||||
@db_api.retry_db_errors
|
||||
def dhcp_ready_on_ports(self, context, port_ids):
|
||||
|
|
|
@ -3006,8 +3006,7 @@ class TestDeviceManager(TestConfBase):
|
|||
|
||||
def test__setup_reserved_dhcp_port_with_fake_remote_error(self):
|
||||
"""Test scenario where a fake_network has two reserved ports, and
|
||||
update_dhcp_port fails for the first of those with a RemoteError
|
||||
different than DhcpPortInUse.
|
||||
update_dhcp_port fails for the first of those with a RemoteError.
|
||||
"""
|
||||
# Setup with a reserved DHCP port.
|
||||
fake_network = FakeDualNetworkReserved2()
|
||||
|
@ -3024,31 +3023,6 @@ class TestDeviceManager(TestConfBase):
|
|||
with testtools.ExpectedException(oslo_messaging.RemoteError):
|
||||
dh.setup_dhcp_port(fake_network)
|
||||
|
||||
def test__setup_reserved_dhcp_port_with_known_remote_error(self):
|
||||
"""Test scenario where a fake_network has two reserved ports, and
|
||||
update_dhcp_port fails for the first of those with a DhcpPortInUse
|
||||
RemoteError.
|
||||
"""
|
||||
# Setup with a reserved DHCP port.
|
||||
fake_network = FakeDualNetworkReserved2()
|
||||
fake_network.tenant_id = 'Tenant A'
|
||||
reserved_port_1 = fake_network.ports[-2]
|
||||
reserved_port_2 = fake_network.ports[-1]
|
||||
|
||||
mock_plugin = mock.Mock()
|
||||
dh = dhcp.DeviceManager(cfg.CONF, mock_plugin)
|
||||
messaging_error = oslo_messaging.RemoteError(exc_type='DhcpPortInUse')
|
||||
mock_plugin.update_dhcp_port.side_effect = [messaging_error,
|
||||
reserved_port_2]
|
||||
|
||||
with mock.patch.object(dhcp.LOG, 'info') as log:
|
||||
dh.setup_dhcp_port(fake_network)
|
||||
self.assertEqual(1, log.call_count)
|
||||
expected_calls = [mock.call(reserved_port_1.id, mock.ANY),
|
||||
mock.call(reserved_port_2.id, mock.ANY)]
|
||||
self.assertEqual(expected_calls,
|
||||
mock_plugin.update_dhcp_port.call_args_list)
|
||||
|
||||
|
||||
class TestDictModel(base.BaseTestCase):
|
||||
|
||||
|
|
|
@ -21,6 +21,7 @@ from neutron_lib import exceptions
|
|||
from neutron_lib.plugins import constants as plugin_constants
|
||||
from neutron_lib.plugins import directory
|
||||
from oslo_db import exception as db_exc
|
||||
from oslo_messaging.rpc import dispatcher as rpc_dispatcher
|
||||
|
||||
from neutron.api.rpc.handlers import dhcp_rpc
|
||||
from neutron.common import utils
|
||||
|
@ -306,12 +307,9 @@ class TestDhcpRpcCallback(base.BaseTestCase):
|
|||
|
||||
self.plugin.get_port.return_value = {
|
||||
'device_id': 'other_id'}
|
||||
self.assertRaises(exceptions.DhcpPortInUse,
|
||||
self.callbacks.update_dhcp_port,
|
||||
mock.Mock(),
|
||||
host='foo_host',
|
||||
port_id='foo_port_id',
|
||||
port=port)
|
||||
res = self.callbacks.update_dhcp_port(mock.Mock(), host='foo_host',
|
||||
port_id='foo_port_id', port=port)
|
||||
self.assertIsNone(res)
|
||||
|
||||
def test_update_dhcp_port(self):
|
||||
port = {'port': {'network_id': 'foo_network_id',
|
||||
|
@ -342,7 +340,7 @@ class TestDhcpRpcCallback(base.BaseTestCase):
|
|||
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.assertRaises(rpc_dispatcher.ExpectedException,
|
||||
self.callbacks.update_dhcp_port,
|
||||
mock.Mock(),
|
||||
host='foo_host',
|
||||
|
|
Loading…
Reference in New Issue