From: Eugene Nikanorov Date: Mon, 19 Oct 2015 13:41:32 +0000 (+0400) Subject: Avoid race condition for reserved DHCP ports X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=f76ef76f2516dad794818ce56fb15d16437f7314;p=openstack-build%2Fneutron-build.git Avoid race condition for reserved DHCP ports This patch introduces mechanism similar to compare-and-swap for updating reserved DHCP port. This addresses a case when two DHCP agents that start nearly at the same time are assigned to one network and there is a reserved DHCP port in the network. Then each of agents will try to use it because agents don't check if reserved port is still available. Reserved DHCP port can be acquired by different agent between calls to get_active_networks and update_port, so this patch adds a check for this case. Change-Id: I0277ab537ff9d3a664c03ea291b9ec2b0e784dbb Closes-Bug: #1425402 --- diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 2f3c87b6d..00eb744cf 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -23,6 +23,7 @@ import time import netaddr from oslo_config import cfg from oslo_log import log as logging +import oslo_messaging from oslo_utils import uuidutils import six @@ -1092,9 +1093,16 @@ 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: - port = self.plugin.update_dhcp_port( - port.id, {'port': {'network_id': network.id, - 'device_id': device_id}}) + 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 == exceptions.DhcpPortInUse: + LOG.info(_LI("Skipping DHCP port %s as it is " + "already in use"), port.id) + continue + raise if port: return port diff --git a/neutron/api/rpc/handlers/dhcp_rpc.py b/neutron/api/rpc/handlers/dhcp_rpc.py index d7388e9e0..959e006c0 100644 --- a/neutron/api/rpc/handlers/dhcp_rpc.py +++ b/neutron/api/rpc/handlers/dhcp_rpc.py @@ -214,10 +214,15 @@ class DhcpRpcCallback(object): host = kwargs.get('host') port = kwargs.get('port') port['id'] = kwargs.get('port_id') + port['port'][portbindings.HOST_ID] = host + plugin = manager.NeutronManager.get_plugin() + old_port = plugin.get_port(context, port['id']) + if (old_port['device_id'] != constants.DEVICE_ID_RESERVED_DHCP_PORT + and old_port['device_id'] != + utils.get_dhcp_agent_device_id(port['port']['network_id'], host)): + raise n_exc.DhcpPortInUse(port_id=port['id']) LOG.debug('Update dhcp port %(port)s ' 'from %(host)s.', {'port': port, 'host': host}) - port['port'][portbindings.HOST_ID] = host - plugin = manager.NeutronManager.get_plugin() return self._port_action(plugin, context, port, 'update_port') diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py index 94e52466f..cb8829753 100644 --- a/neutron/common/exceptions.py +++ b/neutron/common/exceptions.py @@ -172,6 +172,10 @@ class ServicePortInUse(InUse): "port API: %(reason)s") +class DhcpPortInUse(InUse): + message = _("Port %(port_id)s is already acquired by another DHCP agent") + + class PortBound(InUse): message = _("Unable to complete operation on port %(port_id)s, " "port is already bound, port type: %(vif_type)s, " 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 0f42bfc7d..9e772bf06 100644 --- a/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py +++ b/neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py @@ -19,6 +19,7 @@ from oslo_db import exception as db_exc from neutron.api.rpc.handlers import dhcp_rpc from neutron.common import constants from neutron.common import exceptions as n_exc +from neutron.common import utils from neutron.tests import base @@ -177,12 +178,46 @@ class TestDhcpRpcCallback(base.BaseTestCase): def _fake_port_action(plugin, context, port, action): self.assertEqual(expected_port, port) + self.plugin.get_port.return_value = { + 'device_id': constants.DEVICE_ID_RESERVED_DHCP_PORT} self.callbacks._port_action = _fake_port_action self.callbacks.update_dhcp_port(mock.Mock(), host='foo_host', port_id='foo_port_id', port=port) + def test_update_reserved_dhcp_port(self): + port = {'port': {'network_id': 'foo_network_id', + 'device_owner': constants.DEVICE_OWNER_DHCP, + 'fixed_ips': [{'subnet_id': 'foo_subnet_id'}]} + } + expected_port = {'port': {'network_id': 'foo_network_id', + 'device_owner': constants.DEVICE_OWNER_DHCP, + 'binding:host_id': 'foo_host', + 'fixed_ips': [{'subnet_id': 'foo_subnet_id'}] + }, + 'id': 'foo_port_id' + } + + def _fake_port_action(plugin, context, port, action): + self.assertEqual(expected_port, port) + + self.plugin.get_port.return_value = { + 'device_id': utils.get_dhcp_agent_device_id('foo_network_id', + 'foo_host')} + self.callbacks._port_action = _fake_port_action + self.callbacks.update_dhcp_port( + mock.Mock(), host='foo_host', port_id='foo_port_id', port=port) + + self.plugin.get_port.return_value = { + 'device_id': 'other_id'} + self.assertRaises(n_exc.DhcpPortInUse, + self.callbacks.update_dhcp_port, + mock.Mock(), + host='foo_host', + port_id='foo_port_id', + port=port) + def test_update_dhcp_port(self): port = {'port': {'network_id': 'foo_network_id', 'device_owner': constants.DEVICE_OWNER_DHCP, @@ -195,6 +230,8 @@ class TestDhcpRpcCallback(base.BaseTestCase): }, 'id': 'foo_port_id' } + self.plugin.get_port.return_value = { + 'device_id': constants.DEVICE_ID_RESERVED_DHCP_PORT} self.callbacks.update_dhcp_port(mock.Mock(), host='foo_host', port_id='foo_port_id',