From 44e4493313f2e24f0c24258e93c5e3904dbaf407 Mon Sep 17 00:00:00 2001 From: armando-migliaccio Date: Tue, 31 Dec 2013 05:51:43 -0800 Subject: [PATCH] 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 --- neutron/plugins/nicira/dbexts/nicira_db.py | 18 ++++- neutron/tests/unit/nicira/test_nicira_db.py | 79 +++++++++++++++++++++ 2 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 neutron/tests/unit/nicira/test_nicira_db.py diff --git a/neutron/plugins/nicira/dbexts/nicira_db.py b/neutron/plugins/nicira/dbexts/nicira_db.py index 5fb2398f7f..10155145b9 100644 --- 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 index 0000000000..accb6fd8e8 --- /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)