From a93d90f25ef5fed9a74962bcc5d128735f8d854d Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Sat, 14 Oct 2023 18:37:43 +0000 Subject: [PATCH] Remove any IPAM allocation if port bulk creation fails During the port bulk creation, if an IPAM allocation fails (for example, if the IP address is outside of the subnet CIDR), the other IPAM allocations already created are deleted before raising the exception. Conflicts: neutron/tests/unit/plugins/ml2/test_plugin.py Closes-Bug: #2039550 Change-Id: I7fd6e38016d099c03f80874bfa1fb8bdaff8bd2c (cherry picked from commit 71a7abb498fded6d8d60453074efdef9657f416e) (cherry picked from commit a4c03671f2b8784cb6bec227ddbd56a16fffbcf1) --- neutron/plugins/ml2/plugin.py | 7 ++-- neutron/tests/unit/plugins/ml2/test_plugin.py | 34 +++++++++++++++++++ ...allocation_leftovers-9d72cc5f616f51e4.yaml | 7 ++++ 3 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/port_bulk_creation_no_ipamallocation_leftovers-9d72cc5f616f51e4.yaml diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 144e279d11d..e9218af5a06 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -1564,10 +1564,9 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, for port in port_list: self._before_create_port(context, port) - port_list, net_cache = self.allocate_macs_and_ips_for_ports( - context, port_list) - try: + port_list, net_cache = self.allocate_macs_and_ips_for_ports( + context, port_list) return self._create_port_bulk(context, port_list, net_cache) except Exception: with excutils.save_and_reraise_exception(): @@ -1575,7 +1574,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, # deallocated now for port in port_list: self.ipam.deallocate_ips_from_port( - context, port, port['ipams']) + context, port, port.get('ipams')) @db_api.retry_if_session_inactive() def _create_port_bulk(self, context, port_list, network_cache): diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index 6a463344422..d10c91d3607 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -55,6 +55,7 @@ from neutron.db import ipam_pluggable_backend from neutron.db import provisioning_blocks from neutron.db import securitygroups_db as sg_db from neutron.db import segments_db +from neutron.ipam import driver from neutron.objects import base as base_obj from neutron.objects import ports as port_obj from neutron.objects import router as l3_obj @@ -1699,6 +1700,39 @@ class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase): ports_out = self.plugin.create_port_bulk(ctx, ports_in) self.assertEqual(edo, ports_out[0]['extra_dhcp_opts']) + def test_create_ports_bulk_with_wrong_fixed_ips(self): + cidr = '10.0.10.0/24' + with self.network() as net: + with self.subnet(net, cidr=cidr) as snet: + net_id = net['network']['id'] + data = [{'network_id': net_id, + 'fixed_ips': [{'subnet_id': snet['subnet']['id'], + 'ip_address': '10.0.10.100'}], + 'tenant_id': snet['subnet']['tenant_id'] + }, + {'network_id': net_id, + 'fixed_ips': [{'subnet_id': snet['subnet']['id'], + 'ip_address': '10.0.20.101'}], + 'tenant_id': snet['subnet']['tenant_id'] + }] + res = self._create_bulk_from_list(self.fmt, 'port', + data, as_admin=True) + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) + self.assertIn('IP address 10.0.20.101 is not a valid IP for ' + 'the specified subnet.', + res.json['NeutronError']['message']) + + ipam_driver = driver.Pool.get_instance(None, self.context) + ipam_allocator = ipam_driver.get_allocator([cidr]) + with db_api.CONTEXT_READER.using(self.context): + ipam_subnet = ipam_allocator._driver.get_subnet( + snet['subnet']['id']) + allocations = ipam_subnet.subnet_manager.list_allocations( + self.context) + # There are no leftovers (e.g.: 10.0.10.100) in the + # "IpamAllocation" registers + self.assertEqual([], allocations) + def test_delete_port_no_notify_in_disassociate_floatingips(self): ctx = context.get_admin_context() plugin = directory.get_plugin() diff --git a/releasenotes/notes/port_bulk_creation_no_ipamallocation_leftovers-9d72cc5f616f51e4.yaml b/releasenotes/notes/port_bulk_creation_no_ipamallocation_leftovers-9d72cc5f616f51e4.yaml new file mode 100644 index 00000000000..806f2d416ce --- /dev/null +++ b/releasenotes/notes/port_bulk_creation_no_ipamallocation_leftovers-9d72cc5f616f51e4.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + During the port bulk creation, if an IPAM allocation fails (for example, if + the IP address is outside of the subnet CIDR), the other IPAM allocations + already created are deleted before raising the exception. Fixes bug + `2039550 `_.