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.
Conflicts:
neutron/api/rpc/handlers/dhcp_rpc.py
Closes-Bug: #1829332
Change-Id: If7a7ac2f88ce5b0e799c1104c936735a6cc860aa
(cherry picked from commit 494b65d951
)
This commit is contained in:
parent
7606aeec73
commit
1146526304
@ -162,7 +162,8 @@ class DhcpAgent(manager.Manager):
|
|||||||
if (isinstance(e, oslo_messaging.RemoteError) and
|
if (isinstance(e, oslo_messaging.RemoteError) and
|
||||||
e.exc_type == 'NetworkNotFound' or
|
e.exc_type == 'NetworkNotFound' or
|
||||||
isinstance(e, exceptions.NetworkNotFound)):
|
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:
|
else:
|
||||||
LOG.exception('Unable to %(action)s dhcp for %(net_id)s.',
|
LOG.exception('Unable to %(action)s dhcp for %(net_id)s.',
|
||||||
{'net_id': network.id, 'action': action})
|
{'net_id': network.id, 'action': action})
|
||||||
|
@ -26,7 +26,6 @@ from neutron_lib import constants
|
|||||||
from neutron_lib import exceptions
|
from neutron_lib import exceptions
|
||||||
from neutron_lib.utils import file as file_utils
|
from neutron_lib.utils import file as file_utils
|
||||||
from oslo_log import log as logging
|
from oslo_log import log as logging
|
||||||
import oslo_messaging
|
|
||||||
from oslo_utils import excutils
|
from oslo_utils import excutils
|
||||||
from oslo_utils import fileutils
|
from oslo_utils import fileutils
|
||||||
from oslo_utils import uuidutils
|
from oslo_utils import uuidutils
|
||||||
@ -1368,16 +1367,9 @@ class DeviceManager(object):
|
|||||||
for port in network.ports:
|
for port in network.ports:
|
||||||
port_device_id = getattr(port, 'device_id', None)
|
port_device_id = getattr(port, 'device_id', None)
|
||||||
if port_device_id == constants.DEVICE_ID_RESERVED_DHCP_PORT:
|
if port_device_id == constants.DEVICE_ID_RESERVED_DHCP_PORT:
|
||||||
try:
|
port = self.plugin.update_dhcp_port(
|
||||||
port = self.plugin.update_dhcp_port(
|
port.id, {'port': {'network_id': network.id,
|
||||||
port.id, {'port': {'network_id': network.id,
|
'device_id': device_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
|
|
||||||
if port:
|
if port:
|
||||||
return port
|
return port
|
||||||
|
|
||||||
|
@ -33,7 +33,6 @@ from oslo_utils import excutils
|
|||||||
|
|
||||||
from neutron._i18n import _
|
from neutron._i18n import _
|
||||||
from neutron.common import constants as n_const
|
from neutron.common import constants as n_const
|
||||||
from neutron.common import exceptions as n_exc
|
|
||||||
from neutron.common import utils
|
from neutron.common import utils
|
||||||
from neutron.db import provisioning_blocks
|
from neutron.db import provisioning_blocks
|
||||||
from neutron.extensions import segment as segment_ext
|
from neutron.extensions import segment as segment_ext
|
||||||
@ -280,6 +279,7 @@ class DhcpRpcCallback(object):
|
|||||||
hosts=[host])
|
hosts=[host])
|
||||||
return len(agents) != 0
|
return len(agents) != 0
|
||||||
|
|
||||||
|
@oslo_messaging.expected_exceptions(exceptions.NetworkNotFound)
|
||||||
@oslo_messaging.expected_exceptions(exceptions.IpAddressGenerationFailure)
|
@oslo_messaging.expected_exceptions(exceptions.IpAddressGenerationFailure)
|
||||||
@db_api.retry_db_errors
|
@db_api.retry_db_errors
|
||||||
def update_dhcp_port(self, context, **kwargs):
|
def update_dhcp_port(self, context, **kwargs):
|
||||||
@ -295,10 +295,14 @@ class DhcpRpcCallback(object):
|
|||||||
if (old_port['device_id'] !=
|
if (old_port['device_id'] !=
|
||||||
constants.DEVICE_ID_RESERVED_DHCP_PORT and
|
constants.DEVICE_ID_RESERVED_DHCP_PORT and
|
||||||
old_port['device_id'] !=
|
old_port['device_id'] !=
|
||||||
utils.get_dhcp_agent_device_id(network_id, host) or
|
utils.get_dhcp_agent_device_id(network_id, host)):
|
||||||
not self._is_dhcp_agent_hosting_network(plugin, context, host,
|
return
|
||||||
network_id)):
|
if not self._is_dhcp_agent_hosting_network(plugin, context, host,
|
||||||
raise n_exc.DhcpPortInUse(port_id=port['id'])
|
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 '
|
LOG.debug('Update dhcp port %(port)s '
|
||||||
'from %(host)s.',
|
'from %(host)s.',
|
||||||
{'port': port,
|
{'port': port,
|
||||||
@ -308,7 +312,6 @@ class DhcpRpcCallback(object):
|
|||||||
LOG.debug('Host %(host)s tried to update port '
|
LOG.debug('Host %(host)s tried to update port '
|
||||||
'%(port_id)s which no longer exists.',
|
'%(port_id)s which no longer exists.',
|
||||||
{'host': host, 'port_id': port['id']})
|
{'host': host, 'port_id': port['id']})
|
||||||
return None
|
|
||||||
|
|
||||||
@db_api.retry_db_errors
|
@db_api.retry_db_errors
|
||||||
def dhcp_ready_on_ports(self, context, port_ids):
|
def dhcp_ready_on_ports(self, context, port_ids):
|
||||||
|
@ -2949,8 +2949,7 @@ class TestDeviceManager(TestConfBase):
|
|||||||
|
|
||||||
def test__setup_reserved_dhcp_port_with_fake_remote_error(self):
|
def test__setup_reserved_dhcp_port_with_fake_remote_error(self):
|
||||||
"""Test scenario where a fake_network has two reserved ports, and
|
"""Test scenario where a fake_network has two reserved ports, and
|
||||||
update_dhcp_port fails for the first of those with a RemoteError
|
update_dhcp_port fails for the first of those with a RemoteError.
|
||||||
different than DhcpPortInUse.
|
|
||||||
"""
|
"""
|
||||||
# Setup with a reserved DHCP port.
|
# Setup with a reserved DHCP port.
|
||||||
fake_network = FakeDualNetworkReserved2()
|
fake_network = FakeDualNetworkReserved2()
|
||||||
@ -2967,31 +2966,6 @@ class TestDeviceManager(TestConfBase):
|
|||||||
with testtools.ExpectedException(oslo_messaging.RemoteError):
|
with testtools.ExpectedException(oslo_messaging.RemoteError):
|
||||||
dh.setup_dhcp_port(fake_network)
|
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):
|
class TestDictModel(base.BaseTestCase):
|
||||||
|
|
||||||
|
@ -21,9 +21,9 @@ from neutron_lib import exceptions as n_exc
|
|||||||
from neutron_lib.plugins import constants as plugin_constants
|
from neutron_lib.plugins import constants as plugin_constants
|
||||||
from neutron_lib.plugins import directory
|
from neutron_lib.plugins import directory
|
||||||
from oslo_db import exception as db_exc
|
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.api.rpc.handlers import dhcp_rpc
|
||||||
from neutron.common import exceptions
|
|
||||||
from neutron.common import utils
|
from neutron.common import utils
|
||||||
from neutron.db import provisioning_blocks
|
from neutron.db import provisioning_blocks
|
||||||
from neutron.tests import base
|
from neutron.tests import base
|
||||||
@ -304,12 +304,9 @@ class TestDhcpRpcCallback(base.BaseTestCase):
|
|||||||
|
|
||||||
self.plugin.get_port.return_value = {
|
self.plugin.get_port.return_value = {
|
||||||
'device_id': 'other_id'}
|
'device_id': 'other_id'}
|
||||||
self.assertRaises(exceptions.DhcpPortInUse,
|
res = self.callbacks.update_dhcp_port(mock.Mock(), host='foo_host',
|
||||||
self.callbacks.update_dhcp_port,
|
port_id='foo_port_id', port=port)
|
||||||
mock.Mock(),
|
self.assertIsNone(res)
|
||||||
host='foo_host',
|
|
||||||
port_id='foo_port_id',
|
|
||||||
port=port)
|
|
||||||
|
|
||||||
def test_update_dhcp_port(self):
|
def test_update_dhcp_port(self):
|
||||||
port = {'port': {'network_id': 'foo_network_id',
|
port = {'port': {'network_id': 'foo_network_id',
|
||||||
@ -340,7 +337,7 @@ class TestDhcpRpcCallback(base.BaseTestCase):
|
|||||||
self.plugin.get_port.return_value = {
|
self.plugin.get_port.return_value = {
|
||||||
'device_id': constants.DEVICE_ID_RESERVED_DHCP_PORT}
|
'device_id': constants.DEVICE_ID_RESERVED_DHCP_PORT}
|
||||||
self.mock_agent_hosting_network.return_value = False
|
self.mock_agent_hosting_network.return_value = False
|
||||||
self.assertRaises(exceptions.DhcpPortInUse,
|
self.assertRaises(rpc_dispatcher.ExpectedException,
|
||||||
self.callbacks.update_dhcp_port,
|
self.callbacks.update_dhcp_port,
|
||||||
mock.Mock(),
|
mock.Mock(),
|
||||||
host='foo_host',
|
host='foo_host',
|
||||||
|
Loading…
Reference in New Issue
Block a user