nicira: fix db integrity error during port deletion

Due to the fact that plugin port operations are not
transactional (as they involve non synchronized DB
and Controller operations), concurrent (interleaved)
port requests may accidentally cause the insertion
of the mapping entry (neutron-port-id, nvp-port-id)
more than once. In case this occurs, it's safe to
expect the failure and continue the normal process
of the operation being requested.

Closes-bug: #1265081

Change-Id: Ifcf5b453fa08145df844c2de3cbb08bf2f4baa59

diff --git a/neutron/plugins/nicira/dbexts/nicira_db.py b/neutron/plugins/nicira/dbexts/nicira_db.py
index 5fb2398f7f9084260ee17b4077a254c20dc0621d..10155145b91216c92bb377624b78ba6d1e157201 100644 (file)
--- a/neutron/plugins/nicira/dbexts/nicira_db.py
+++ b/neutron/plugins/nicira/dbexts/nicira_db.py
@@ -18,6 +18,7 @@
 from sqlalchemy.orm import exc
 
 import neutron.db.api as db
+from neutron.openstack.common.db import exception as d_exc
 from neutron.openstack.common import log as logging
 from neutron.plugins.nicira.dbexts import nicira_models
 from neutron.plugins.nicira.dbexts import nicira_networkgw_db
@@ -49,11 +50,24 @@ def add_network_binding(session, network_id, binding_type, phy_uuid, vlan_id):
 
 def add_neutron_nsx_port_mapping(session, neutron_id,
                                  nsx_switch_id, nsx_port_id):
-    with session.begin(subtransactions=True):
+    session.begin(subtransactions=True)
+    try:
         mapping = nicira_models.NeutronNsxPortMapping(
             neutron_id, nsx_switch_id, nsx_port_id)
         session.add(mapping)
-        return mapping
+        session.commit()
+    except d_exc.DBDuplicateEntry:
+        session.rollback()
+        # do not complain if the same exact mapping is being added, otherwise
+        # re-raise because even though it is possible for the same neutron
+        # port to map to different back-end ports over time, this should not
+        # occur whilst a mapping already exists
+        current = get_nsx_switch_and_port_id(session, neutron_id)
+        if current[1] == nsx_port_id:
+            LOG.debug(_("Port mapping for %s already available"), neutron_id)
+        else:
+            raise
+    return mapping
 
 
 def get_nsx_switch_and_port_id(session, neutron_id):

diff --git a/neutron/tests/unit/nicira/test_nicira_db.py b/neutron/tests/unit/nicira/test_nicira_db.py
new file mode 100644 (file)
index 0000000..accb6fd
--- /dev/null
+++ b/neutron/tests/unit/nicira/test_nicira_db.py
@@ -0,0 +1,79 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2013 VMware, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from neutron import context
+from neutron.db import api as db
+from neutron.db import models_v2
+from neutron.openstack.common.db import exception as d_exc
+from neutron.plugins.nicira.dbexts import nicira_db
+from neutron.plugins.nicira.dbexts import nicira_models
+from neutron.tests import base
+
+
+class NiciraDBTestCase(base.BaseTestCase):
+
+    def setUp(self):
+        super(NiciraDBTestCase, self).setUp()
+        db.configure_db()
+        self.ctx = context.get_admin_context()
+        self.addCleanup(db.clear_db)
+
+    def _setup_neutron_network_and_port(self, network_id, port_id):
+        with self.ctx.session.begin(subtransactions=True):
+            self.ctx.session.add(models_v2.Network(id=network_id))
+            port = models_v2.Port(id=port_id,
+                                  network_id=network_id,
+                                  mac_address='foo_mac_address',
+                                  admin_state_up=True,
+                                  status='ACTIVE',
+                                  device_id='',
+                                  device_owner='')
+            self.ctx.session.add(port)
+
+    def test_add_neutron_nsx_port_mapping_handle_duplicate_constraint(self):
+        neutron_net_id = 'foo_neutron_network_id'
+        neutron_port_id = 'foo_neutron_port_id'
+        nsx_port_id = 'foo_nsx_port_id'
+        nsx_switch_id = 'foo_nsx_switch_id'
+        self._setup_neutron_network_and_port(neutron_net_id, neutron_port_id)
+
+        nicira_db.add_neutron_nsx_port_mapping(
+            self.ctx.session, neutron_port_id, nsx_switch_id, nsx_port_id)
+        # Call the method twice to trigger a db duplicate constraint error
+        nicira_db.add_neutron_nsx_port_mapping(
+            self.ctx.session, neutron_port_id, nsx_switch_id, nsx_port_id)
+        result = (self.ctx.session.query(nicira_models.NeutronNsxPortMapping).
+                  filter_by(neutron_id=neutron_port_id).one())
+        self.assertEqual(nsx_port_id, result.nsx_port_id)
+        self.assertEqual(neutron_port_id, result.neutron_id)
+
+    def test_add_neutron_nsx_port_mapping_raise_on_duplicate_constraint(self):
+        neutron_net_id = 'foo_neutron_network_id'
+        neutron_port_id = 'foo_neutron_port_id'
+        nsx_port_id_1 = 'foo_nsx_port_id_1'
+        nsx_port_id_2 = 'foo_nsx_port_id_2'
+        nsx_switch_id = 'foo_nsx_switch_id'
+        self._setup_neutron_network_and_port(neutron_net_id, neutron_port_id)
+
+        nicira_db.add_neutron_nsx_port_mapping(
+            self.ctx.session, neutron_port_id, nsx_switch_id, nsx_port_id_1)
+        # Call the method twice to trigger a db duplicate constraint error,
+        # this time with a different nsx port id!
+        self.assertRaises(d_exc.DBDuplicateEntry,
+                          nicira_db.add_neutron_nsx_port_mapping,
+                          self.ctx.session, neutron_port_id,
+                          nsx_switch_id, nsx_port_id_2)