Merge "Allocate IPs in bulk requests in separate transactions" into stable/train

neutron/db/ipam_pluggable_backend.py

@@ -179,6 +179,24 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin):
         ipam_driver = driver.Pool.get_instance(None, context)
         return ipam_driver.get_subnet(subnet_id)
 
+    @db_api.retry_if_session_inactive()
+    @db_api.CONTEXT_WRITER
+    def deallocate_ips_from_port(self, context, port, ips):
+        """Deallocate set of ips from port.
+
+        Deallocate IP addresses previosly allocated for given port.
+        Format of the ips:
+            [{
+                "ip_address": IP.ADDRESS,
+                "subnet_id": subnet_id
+            }]
+        """
+        if not ips:
+            return
+        ipam_driver = driver.Pool.get_instance(None, context)
+        self._ipam_deallocate_ips(
+            context, ipam_driver, port['port'], ips)
+
     def allocate_ips_for_port_and_store(self, context, port, port_id):
         # Make a copy of port dict to prevent changing
         # incoming dict by adding 'id' to it.
@@ -198,28 +216,34 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin):
 
         port_copy['port']['id'] = port_id
         network_id = port_copy['port']['network_id']
-        ips = []
+        ips = self.allocate_ips_for_port(context, port_copy)
+        self.store_ip_allocation_for_port(context, ips, network_id, port_copy)
+        return ips
+
+    @db_api.retry_if_session_inactive()
+    @db_api.CONTEXT_WRITER
+    def allocate_ips_for_port(self, context, port):
+        return self._allocate_ips_for_port(context, port)
+
+    def store_ip_allocation_for_port(self, context, ips, network_id, port):
         try:
-            ips = self._allocate_ips_for_port(context, port_copy)
             for ip in ips:
                 ip_address = ip['ip_address']
                 subnet_id = ip['subnet_id']
                 IpamPluggableBackend._store_ip_allocation(
                     context, ip_address, network_id,
-                    subnet_id, port_id)
-            return ips
+                    subnet_id, port['port']['id'])
         except Exception:
             with excutils.save_and_reraise_exception():
-                if ips:
-                    ipam_driver = driver.Pool.get_instance(None, context)
-                    if not ipam_driver.needs_rollback():
-                        return
+                ipam_driver = driver.Pool.get_instance(None, context)
+                if not ipam_driver.needs_rollback():
+                    return
 
-                    LOG.debug("An exception occurred during port creation. "
-                              "Reverting IP allocation")
-                    self._safe_rollback(self._ipam_deallocate_ips, context,
-                                        ipam_driver, port_copy['port'], ips,
-                                        revert_on_fail=False)
+                LOG.debug("An exception occurred during port creation. "
+                          "Reverting IP allocation")
+                self._safe_rollback(self._ipam_deallocate_ips, context,
+                                    ipam_driver, port['port'], ips,
+                                    revert_on_fail=False)
 
     def _allocate_ips_for_port(self, context, port):
         """Allocate IP addresses for the port. IPAM version.

neutron/plugins/ml2/plugin.py

@@ -1457,14 +1457,43 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         sgids = self._get_security_groups_on_port(context, port_compat)
         self._process_port_create_security_group(context, port_dict, sgids)
 
+    def allocate_ips_for_ports(self, context, ports):
+        for port in ports:
+            port['port']['id'] = (
+                port['port'].get('id') or uuidutils.generate_uuid())
+
+            # Call IPAM to allocate IP addresses
+            try:
+                port['ipams'] = self.ipam.allocate_ips_for_port(context, port)
+
+                port['ip_allocation'] = (ipalloc_apidef.
+                                         IP_ALLOCATION_IMMEDIATE)
+            except ipam_exc.DeferIpam:
+                port['ip_allocation'] = (ipalloc_apidef.
+                                         IP_ALLOCATION_DEFERRED)
+        return ports
+
     @utils.transaction_guard
-    @db_api.retry_if_session_inactive()
     def create_port_bulk(self, context, ports):
-        # TODO(njohnston): Break this up into smaller functions.
         port_list = ports.get('ports')
         for port in port_list:
             self._before_create_port(context, port)
 
+        port_list = self.allocate_ips_for_ports(context, port_list)
+
+        try:
+            return self._create_port_bulk(context, port_list)
+        except Exception:
+            with excutils.save_and_reraise_exception():
+                # If any issue happened allocated IP addresses needs to be
+                # deallocated now
+                for port in port_list:
+                    self.ipam.deallocate_ips_from_port(
+                        context, port, port['ipams'])
+
+    @db_api.retry_if_session_inactive()
+    def _create_port_bulk(self, context, port_list):
+        # TODO(njohnston): Break this up into smaller functions.
         port_data = []
         network_cache = dict()
         macs = self._generate_macs(len(port_list))
@@ -1517,31 +1546,25 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                 # Create the Port object
                 db_port_obj = ports_obj.Port(context,
                                             mac_address=eui_mac_address,
-                                            id=uuidutils.generate_uuid(),
+                                            id=port['port']['id'],
                                             **bulk_port_data)
                 db_port_obj.create()
 
-                # Call IPAM to allocate IP addresses
-                try:
-                    # TODO(njohnston): IPAM allocation needs to be revamped to
-                    # be bulk-friendly.
-                    ips = self.ipam.allocate_ips_for_port_and_store(
-                            context, port, db_port_obj['id'])
-                    ipam_fixed_ips = []
-                    for ip in ips:
-                        fixed_ip = ports_obj.IPAllocation(
-                                port_id=db_port_obj['id'],
-                                subnet_id=ip['subnet_id'],
-                                network_id=network_id,
-                                ip_address=ip['ip_address'])
-                        ipam_fixed_ips.append(fixed_ip)
-
-                    db_port_obj['fixed_ips'] = ipam_fixed_ips
-                    db_port_obj['ip_allocation'] = (ipalloc_apidef.
-                                                IP_ALLOCATION_IMMEDIATE)
-                except ipam_exc.DeferIpam:
-                    db_port_obj['ip_allocation'] = (ipalloc_apidef.
-                                                IP_ALLOCATION_DEFERRED)
+                # Call IPAM to store allocated IP addresses
+                ipams = port.pop("ipams")
+                self.ipam.store_ip_allocation_for_port(
+                    context, ipams, network_id, port)
+                ipam_fixed_ips = []
+                for ip in ipams:
+                    fixed_ip = ports_obj.IPAllocation(
+                            port_id=db_port_obj['id'],
+                            subnet_id=ip['subnet_id'],
+                            network_id=network_id,
+                            ip_address=ip['ip_address'])
+                    ipam_fixed_ips.append(fixed_ip)
+
+                db_port_obj['fixed_ips'] = ipam_fixed_ips
+                db_port_obj['ip_allocation'] = port.pop('ip_allocation')
 
                 fixed_ips = pdata.get('fixed_ips')
                 if validators.is_attr_set(fixed_ips) and not fixed_ips:

neutron/tests/unit/plugins/ml2/test_plugin.py

@@ -1420,6 +1420,27 @@ class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase):
                                             data, context=ctx)
                 self.assertFalse(m_upd.called)
 
+    def test_create_ports_bulk_ip_allocation_reverted_in_case_of_error(self):
+        ctx = context.get_admin_context()
+        with self.network() as net:
+            plugin = directory.get_plugin()
+            with mock.patch.object(
+                plugin, '_create_port_bulk',
+                side_effect=ml2_exc.MechanismDriverError(
+                    method='create_port_bulk')), \
+                    mock.patch.object(
+                        plugin.ipam,
+                        'deallocate_ips_from_port') as deallocate_mock:
+
+                res = self._create_port_bulk(self.fmt, 2, net['network']['id'],
+                                             'test', True, context=ctx)
+
+                # We expect a 500 as we injected a fault in the plugin
+                self._validate_behavior_on_bulk_failure(
+                    res, 'ports', webob.exc.HTTPServerError.code)
+
+                self.assertEqual(2, deallocate_mock.call_count)
+
     def test_delete_port_no_notify_in_disassociate_floatingips(self):
         ctx = context.get_admin_context()
         plugin = directory.get_plugin()