diff --git a/doc/source/devref/calling_ml2_plugin.rst b/doc/source/devref/calling_ml2_plugin.rst new file mode 100644 index 00000000000..6cae36362bf --- /dev/null +++ b/doc/source/devref/calling_ml2_plugin.rst @@ -0,0 +1,47 @@ +.. + 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. + + + Convention for heading levels in Neutron devref: + ======= Heading 0 (reserved for the title in a document) + ------- Heading 1 + ~~~~~~~ Heading 2 + +++++++ Heading 3 + ''''''' Heading 4 + (Avoid deeper levels because they do not render well.) + + +Calling the ML2 Plugin +====================== + +When writing code for an extension, service plugin, or any other part of +Neutron you must not call core plugin methods that mutate state while +you have a transaction open on the session that you pass into the core +plugin method. + +The create and update methods for ports, networks, and subnets in ML2 +all have a precommit phase and postcommit phase. During the postcommit +phase, the data is expected to be fully persisted to the database and +ML2 drivers will use this time to relay information to a backend outside +of Neutron. Calling the ML2 plugin within a transaction would violate +this semantic because the data would not be persisted to the DB; and, +were a failure to occur that caused the whole transaction to be rolled +back, the backend would become inconsistent with the state in Neutron's +DB. + +To prevent this, these methods are protected with a decorator that will +raise a RuntimeError if they are called with context that has a session +in an active transaction. The decorator can be found at +neutron.common.utils.transaction_guard and may be used in other places +in Neutron to protect functions that are expected to be called outside +of a transaction. diff --git a/doc/source/devref/index.rst b/doc/source/devref/index.rst index 72581ce3215..bbb84d5ab73 100644 --- a/doc/source/devref/index.rst +++ b/doc/source/devref/index.rst @@ -55,6 +55,7 @@ Neutron Internals services_and_agents api_layer ml2_ext_manager + calling_ml2_plugin quota api_extensions plugin-api diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 01348e24a75..2c616b62a5f 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -757,6 +757,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, self._apply_dict_extend_functions('networks', result, net_db) return result, mech_context + @utils.transaction_guard def create_network(self, context, network): result, mech_context = self._create_network_db(context, network) kwargs = {'context': context, 'network': result} @@ -771,10 +772,12 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, return result + @utils.transaction_guard def create_network_bulk(self, context, networks): objects = self._create_bulk_ml2(attributes.NETWORK, context, networks) return [obj['result'] for obj in objects] + @utils.transaction_guard def update_network(self, context, id, network): net_data = network[attributes.NETWORK] provider._raise_if_updates_provider_attributes(net_data) @@ -966,6 +969,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, return result, mech_context + @utils.transaction_guard def create_subnet(self, context, subnet): result, mech_context = self._create_subnet_db(context, subnet) kwargs = {'context': context, 'subnet': result} @@ -979,10 +983,12 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, self.delete_subnet(context, result['id']) return result + @utils.transaction_guard def create_subnet_bulk(self, context, subnets): objects = self._create_bulk_ml2(attributes.SUBNET, context, subnets) return [obj['result'] for obj in objects] + @utils.transaction_guard def update_subnet(self, context, id, subnet): session = context.session with session.begin(subtransactions=True): @@ -1256,6 +1262,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, return bound_context.current + @utils.transaction_guard def create_port_bulk(self, context, ports): objects = self._create_bulk_ml2(attributes.PORT, context, ports) @@ -1462,6 +1469,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, binding.host = attrs and attrs.get(portbindings.HOST_ID) binding.router_id = attrs and attrs.get('device_id') + @utils.transaction_guard def update_distributed_port_binding(self, context, id, port): attrs = port[attributes.PORT] @@ -1518,6 +1526,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, raise e.errors[0].error raise exc.ServicePortInUse(port_id=port_id, reason=e) + @utils.transaction_guard def delete_port(self, context, id, l3_port_check=True): self._pre_delete_port(context, id, l3_port_check) # TODO(armax): get rid of the l3 dependency in the with block @@ -1587,6 +1596,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, self.notifier.port_delete(context, port['id']) self.notify_security_groups_member_updated(context, port) + @utils.transaction_guard def get_bound_port_context(self, plugin_context, port_id, host=None, cached_networks=None): session = plugin_context.session @@ -1643,6 +1653,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, exception_checker=lambda e: isinstance(e, (sa_exc.StaleDataError, os_db_exception.DBDeadlock)) ) + @utils.transaction_guard def update_port_status(self, context, port_id, status, host=None, network=None): """ diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index 5012218f478..f833cbd618d 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -1565,20 +1565,16 @@ class TestMl2PortBinding(Ml2PluginV2TestCase, def test_update_distributed_port_binding_on_concurrent_port_delete(self): plugin = manager.NeutronManager.get_plugin() - l3plugin = manager.NeutronManager.get_service_plugins().get( - p_const.L3_ROUTER_NAT) with self.port() as port: port = { 'id': port['port']['id'], portbindings.HOST_ID: 'foo_host', } - # NOTE(rtheis): Mock L3 service prevent_l3_port_deletion() to - # prevent recursive loop introduced by the plugin get_port mock. - with mock.patch.object(plugin, 'get_port', - new=plugin.delete_port), \ - mock.patch.object(l3plugin, 'prevent_l3_port_deletion'): + exc = db_exc.DBReferenceError('', '', '', '') + with mock.patch.object(ml2_db, 'ensure_distributed_port_binding', + side_effect=exc): res = plugin.update_distributed_port_binding( - self.context, 'foo_port_id', {'port': port}) + self.context, port['id'], {'port': port}) self.assertIsNone(res) def test_update_distributed_port_binding_on_non_existent_port(self): @@ -2316,6 +2312,7 @@ class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase): # neutron.objects.db.api from core plugin instance self.setup_coreplugin(PLUGIN_NAME) self.context = mock.MagicMock() + self.context.session.is_active = False self.notify_p = mock.patch('neutron.callbacks.registry.notify') self.notify = self.notify_p.start()