Merge "Avoid race condition for reserved DHCP ports"

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
 
@@ -1096,9 +1097,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
 

neutron/api/rpc/handlers/dhcp_rpc.py

@@ -217,10 +217,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')

neutron/common/exceptions.py

@@ -182,6 +182,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, "

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',