]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Check for concurrent port binding deletion before binding the port
authorEugene Nikanorov <enikanorov@mirantis.com>
Sun, 28 Sep 2014 21:02:58 +0000 (01:02 +0400)
committerenikanorov <enikanorov@mirantis.com>
Wed, 29 Oct 2014 06:07:28 +0000 (06:07 +0000)
When agent tries to update port binding (DVR or regular), the port
might have already been deleted via API call.
This is not an error condition but should be handled to avoid traces
in the logs.

Change-Id: Ie9436172151f0ecd5b3e4667328910b09f8ef141
Closes-Bug: #1370570

neutron/plugins/ml2/plugin.py [changed mode: 0644->0755]
neutron/tests/unit/ml2/test_port_binding.py [changed mode: 0644->0755]

old mode 100644 (file)
new mode 100755 (executable)
index ba60115..6dbedd1
@@ -51,6 +51,7 @@ from neutron.extensions import portbindings
 from neutron.extensions import providernet as provider
 from neutron import manager
 from neutron.openstack.common import excutils
+from neutron.openstack.common.gettextutils import _LI
 from neutron.openstack.common import importutils
 from neutron.openstack.common import jsonutils
 from neutron.openstack.common import lockutils
@@ -948,6 +949,11 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
             cur_binding = db.get_dvr_port_binding_by_host(session,
                                                           port_id,
                                                           host)
+            if not cur_binding:
+                LOG.info(_LI("Binding info for port %s was not found, "
+                             "it might have been deleted already."),
+                         port_id)
+                return
             # Commit our binding results only if port has not been
             # successfully bound concurrently by another thread or
             # process and no binding inputs have been changed.
@@ -1055,6 +1061,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                            filter(models_v2.Port.id.startswith(port_id)).
                            one())
             except sa_exc.NoResultFound:
+                LOG.debug("No ports have port_id starting with %s",
+                          port_id)
                 return
             except exc.MultipleResultsFound:
                 LOG.error(_("Multiple ports have port_id starting with %s"),
@@ -1072,8 +1080,18 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                 port_context = driver_context.DvrPortContext(
                     self, plugin_context, port, network, binding)
             else:
+                # since eager loads are disabled in port_db query
+                # related attribute port_binding could disappear in
+                # concurrent port deletion.
+                # It's not an error condition.
+                binding = port_db.port_binding
+                if not binding:
+                    LOG.info(_LI("Binding info for port %s was not found, "
+                                 "it might have been deleted already."),
+                             port_id)
+                    return
                 port_context = driver_context.PortContext(
-                    self, plugin_context, port, network, port_db.port_binding)
+                    self, plugin_context, port, network, binding)
 
         return self._bind_port_if_needed(port_context)
 
old mode 100644 (file)
new mode 100755 (executable)
index 3f3c4b2..b2da2ab
@@ -19,6 +19,7 @@ from neutron import context
 from neutron.extensions import portbindings
 from neutron import manager
 from neutron.plugins.ml2 import config as config
+from neutron.plugins.ml2 import models as ml2_models
 from neutron.tests.unit import test_db_plugin as test_plugin
 
 
@@ -98,6 +99,29 @@ class PortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase):
                                 portbindings.VIF_TYPE_OVS,
                                 True, True, 'ACTIVE')
 
+    def test_update_port_binding_no_binding(self):
+        ctx = context.get_admin_context()
+        with self.port(name='name') as port:
+            # emulating concurrent binding deletion
+            (ctx.session.query(ml2_models.PortBinding).
+             filter_by(port_id=port['port']['id']).delete())
+            self.assertIsNone(
+                self.plugin.get_bound_port_context(ctx, port['port']['id']))
+
+    def test_commit_dvr_port_binding(self):
+        ctx = context.get_admin_context()
+
+        class MechContext(object):
+            pass
+
+        mctx = MechContext()
+        mctx._binding = None
+        # making a shortcut: calling private method directly to
+        # avoid bothering with "concurrent" port binding deletion
+        res = self.plugin._commit_dvr_port_binding(ctx, 'anyUUID',
+                                                   'HostA', mctx)
+        self.assertIsNone(res)
+
     def _test_update_port_binding(self, host, new_host=None):
         with mock.patch.object(self.plugin,
                                '_notify_port_updated') as notify_mock: