From ee85fd74cf10d27fec6716271d42a8208f1e8694 Mon Sep 17 00:00:00 2001 From: Yang JianFeng Date: Thu, 16 May 2019 17:40:28 +0800 Subject: [PATCH] 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/agent/dhcp/agent.py neutron/api/rpc/handlers/dhcp_rpc.py Closes-Bug: #1829332 Change-Id: If7a7ac2f88ce5b0e799c1104c936735a6cc860aa (cherry picked from commit 494b65d951dbc03f4cebb3bc14970e54ff6adb5f) --- neutron/agent/dhcp/agent.py | 9 +++--- neutron/agent/linux/dhcp.py | 14 ++-------- neutron/api/rpc/handlers/dhcp_rpc.py | 15 ++++++---- neutron/tests/unit/agent/linux/test_dhcp.py | 28 +------------------ .../unit/api/rpc/handlers/test_dhcp_rpc.py | 13 ++++----- 5 files changed, 23 insertions(+), 56 deletions(-) diff --git a/neutron/agent/dhcp/agent.py b/neutron/agent/dhcp/agent.py index 6b03be2d9c5..aeb14902d7a 100644 --- a/neutron/agent/dhcp/agent.py +++ b/neutron/agent/dhcp/agent.py @@ -159,10 +159,11 @@ class DhcpAgent(manager.Manager): # allocation pool or a port is deleted to free up an IP, this # will automatically be retried on the notification self.schedule_resync(e, network.id) - 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) + if (isinstance(e, oslo_messaging.RemoteError) and + e.exc_type == 'NetworkNotFound' or + isinstance(e, exceptions.NetworkNotFound)): + 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}) diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index e8df6078081..a9f1d723586 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -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 @@ -1364,16 +1363,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 diff --git a/neutron/api/rpc/handlers/dhcp_rpc.py b/neutron/api/rpc/handlers/dhcp_rpc.py index 14a71fb6efb..61a5c28db86 100644 --- a/neutron/api/rpc/handlers/dhcp_rpc.py +++ b/neutron/api/rpc/handlers/dhcp_rpc.py @@ -30,7 +30,6 @@ from oslo_utils import excutils from neutron._i18n import _ from neutron.common import constants as n_const -from neutron.common import exceptions as n_exc from neutron.common import utils from neutron.db import api as db_api from neutron.db import provisioning_blocks @@ -279,6 +278,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 +294,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 n_exc.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 +311,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): diff --git a/neutron/tests/unit/agent/linux/test_dhcp.py b/neutron/tests/unit/agent/linux/test_dhcp.py index e3c67993595..756774232f2 100644 --- a/neutron/tests/unit/agent/linux/test_dhcp.py +++ b/neutron/tests/unit/agent/linux/test_dhcp.py @@ -2924,8 +2924,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() @@ -2942,31 +2941,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): diff --git a/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py b/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py index cdc3fb69313..1ed8e869831 100644 --- a/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py +++ b/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py @@ -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 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 exceptions from neutron.common import utils from neutron.db import provisioning_blocks from neutron.tests import base @@ -304,12 +304,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', @@ -340,7 +337,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',