l3_db: it updates port attribute without L2 plugin

L3_NAT_dbonly_mixin._add_interface_by_port update Port.owner
db entry directly without notifying l2 plugin.
Thus L2 plugin/ML2 mechanism driver will be confused when
interface is attached to router by port because port owner is different.
Use L2 plugin update_port method to update port:owner.

Change-Id: If0178887282456842b6078a851a9233cb58a391a
Closes-Bug: #1475093

diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py
index c8dfb2137cd2be1d5b6cceeaf63402513f4d6770..5b10badc89e2aa28e2d40e4c08f92f2fe304ac86 100644 (file)
--- a/neutron/db/l3_db.py
+++ b/neutron/db/l3_db.py
@@ -517,17 +517,29 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
                 raise n_exc.BadRequest(resource='router', msg=msg)
         return port_id_specified, subnet_id_specified
 
+    def _check_router_port(self, context, port_id, device_id):
+        port = self._core_plugin.get_port(context, port_id)
+        if port['device_id'] != device_id:
+            raise n_exc.PortInUse(net_id=port['network_id'],
+                                  port_id=port['id'],
+                                  device_id=port['device_id'])
+        if not port['fixed_ips']:
+            msg = _('Router port must have at least one fixed IP')
+            raise n_exc.BadRequest(resource='router', msg=msg)
+        return port
+
     def _add_interface_by_port(self, context, router, port_id, owner):
+        # Update owner before actual process in order to avoid the
+        # case where a port might get attached to a router without the
+        # owner successfully updating due to an unavailable backend.
+        self._check_router_port(context, port_id, '')
+        self._core_plugin.update_port(
+            context, port_id, {'port': {'device_id': router.id,
+                                        'device_owner': owner}})
+
         with context.session.begin(subtransactions=True):
-            port = self._core_plugin._get_port(context, port_id)
-            if port['device_id']:
-                raise n_exc.PortInUse(net_id=port['network_id'],
-                                      port_id=port['id'],
-                                      device_id=port['device_id'])
-
-            if not port['fixed_ips']:
-                msg = _('Router port must have at least one fixed IP')
-                raise n_exc.BadRequest(resource='router', msg=msg)
+            # check again within transaction to mitigate race
+            port = self._check_router_port(context, port_id, router.id)
 
             # Only allow one router port with IPv6 subnets per network id
             if self._port_has_ipv6_address(port):
@@ -558,8 +570,6 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
                 msg = _("Cannot have multiple "
                         "IPv4 subnets on router port")
                 raise n_exc.BadRequest(resource='router', msg=msg)
-
-            port.update({'device_id': router.id, 'device_owner': owner})
             return port, subnets
 
     def _port_has_ipv6_address(self, port):

diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py
index 9d95a18f0bf23877e2d0619f437c6673c15dde7e..eda6a70d416b2aa10106a7ceb3b73b49a31702a0 100644 (file)
--- a/neutron/tests/unit/extensions/test_l3.py
+++ b/neutron/tests/unit/extensions/test_l3.py
@@ -1175,24 +1175,31 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
                 self.assertIn('port_id', body)
 
     def test_router_add_interface_port(self):
-        with self.router() as r:
-            with self.port() as p:
-                body = self._router_interface_action('add',
-                                                     r['router']['id'],
-                                                     None,
-                                                     p['port']['id'])
-                self.assertIn('port_id', body)
-                self.assertEqual(body['port_id'], p['port']['id'])
+        orig_update_port = self.plugin.update_port
+        with self.router() as r, (
+            self.port()) as p, (
+                mock.patch.object(self.plugin, 'update_port')) as update_port:
+            update_port.side_effect = orig_update_port
+            body = self._router_interface_action('add',
+                                                 r['router']['id'],
+                                                 None,
+                                                 p['port']['id'])
+            self.assertIn('port_id', body)
+            self.assertEqual(p['port']['id'], body['port_id'])
+            expected_port_update = {
+                'device_owner': l3_constants.DEVICE_OWNER_ROUTER_INTF,
+                'device_id': r['router']['id']}
+            update_port.assert_called_with(
+                mock.ANY, p['port']['id'], {'port': expected_port_update})
+            # fetch port and confirm device_id
+            body = self._show('ports', p['port']['id'])
+            self.assertEqual(r['router']['id'], body['port']['device_id'])
 
-                # fetch port and confirm device_id
-                body = self._show('ports', p['port']['id'])
-                self.assertEqual(body['port']['device_id'], r['router']['id'])
-
-                # clean-up
-                self._router_interface_action('remove',
-                                              r['router']['id'],
-                                              None,
-                                              p['port']['id'])
+            # clean-up
+            self._router_interface_action('remove',
+                                          r['router']['id'],
+                                          None,
+                                          p['port']['id'])
 
     def test_router_add_interface_multiple_ipv4_subnet_port_returns_400(self):
         """Test adding router port with multiple IPv4 subnets fails.