From 802cf1194218dc125b7211d9c48df8cfebd4f932 Mon Sep 17 00:00:00 2001 From: Miguel Lavalle Date: Wed, 31 May 2017 16:38:57 -0500 Subject: [PATCH] Fix race between create subnet and port requests When creating an IPv6 auto-address subnet for a network, ports can be created or updated concurrently in the same network, before the subnet creation concludes. In such a situation, an IpAddressAlreadyAllocated exception may be raised, because the subnet create request attempts to allocate auto addresses to the concurrently created or updated ports, which have been already allocated by the port requests. This patchset adds code to the IPAM to skip the attempt to assign the auto address to ports if they already have received them. Change-Id: If1eb4046865f43b15ba97c52e2d0b9343dc72c19 Closes-Bug: #1655567 --- neutron/db/ipam_pluggable_backend.py | 14 +++++++---- .../tests/unit/db/test_db_base_plugin_v2.py | 23 +++++++++++++++---- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/neutron/db/ipam_pluggable_backend.py b/neutron/db/ipam_pluggable_backend.py index dc26346bd60..ea118a8a582 100644 --- a/neutron/db/ipam_pluggable_backend.py +++ b/neutron/db/ipam_pluggable_backend.py @@ -476,12 +476,12 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): LOG.debug("Requesting with IP request: %s port: %s ip: %s " "for subnet %s and ipam_subnet %s", ip_request, port, ip, subnet, ipam_subnet) - ip_address = ipam_subnet.allocate(ip_request) - allocated = models_v2.IPAllocation(network_id=network_id, - port_id=port['id'], - ip_address=ip_address, - subnet_id=subnet['id']) try: + ip_address = ipam_subnet.allocate(ip_request) + allocated = models_v2.IPAllocation(network_id=network_id, + port_id=port['id'], + ip_address=ip_address, + subnet_id=subnet['id']) # Do the insertion of each IP allocation entry within # the context of a nested transaction, so that the entry # is rolled back independently of other entries whenever @@ -499,6 +499,10 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): except Exception: LOG.debug("Reverting IP allocation failed for %s", ip_address) + except ipam_exc.IpAddressAlreadyAllocated: + LOG.debug("Port %s got IPv6 auto-address in a concurrent " + "create or update port request. Ignoring.", + port['id']) return updated_ports def allocate_subnet(self, context, network, subnet, subnetpool_id): diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index d9db78bb697..a55d9bdf555 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -56,6 +56,7 @@ from neutron.db.models import securitygroup as sg_models from neutron.db import models_v2 from neutron.db import rbac_db_models from neutron.db import standard_attr +from neutron.ipam.drivers.neutrondb_ipam import driver as ipam_driver from neutron.ipam import exceptions as ipam_exc from neutron.tests import base from neutron.tests import tools @@ -4342,7 +4343,8 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): def _test_create_subnet_ipv6_auto_addr_with_port_on_network( self, addr_mode, device_owner=DEVICE_OWNER_COMPUTE, - insert_db_reference_error=False, insert_port_not_found=False): + insert_db_reference_error=False, insert_port_not_found=False, + insert_address_allocated=False): # Create a network with one IPv4 subnet and one port with self.network() as network,\ self.subnet(network=network) as v4_subnet,\ @@ -4373,15 +4375,20 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): if insert_port_not_found: mock_updated_port.side_effect = lib_exc.PortNotFound( port_id=port['port']['id']) + if insert_address_allocated: + mock.patch.object( + ipam_driver.NeutronDbSubnet, '_verify_ip', + side_effect=ipam_exc.IpAddressAlreadyAllocated( + subnet_id=mock.ANY, ip=mock.ANY)).start() v6_subnet = self._make_subnet(self.fmt, network, 'fe80::1', 'fe80::/64', ip_version=6, ipv6_ra_mode=addr_mode, ipv6_address_mode=addr_mode) - if (insert_db_reference_error + if (insert_db_reference_error or insert_address_allocated or device_owner == constants.DEVICE_OWNER_ROUTER_SNAT or device_owner in constants.ROUTER_INTERFACE_OWNERS): - # DVR SNAT and router interfaces should not have been - # updated with addresses from the new auto-address subnet + # DVR SNAT, router interfaces and DHCP ports should not have + # been updated with addresses from the new auto-address subnet self.assertEqual(1, len(port['port']['fixed_ips'])) else: # Confirm that the port has been updated with an address @@ -4411,6 +4418,14 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): constants.IPV6_SLAAC, device_owner=constants.DEVICE_OWNER_DHCP) + def test_create_subnet_dhcpv6_stateless_with_ip_already_allocated(self): + self._test_create_subnet_ipv6_auto_addr_with_port_on_network( + constants.DHCPV6_STATELESS, insert_address_allocated=True) + + def test_create_subnet_ipv6_slaac_with_ip_already_allocated(self): + self._test_create_subnet_ipv6_auto_addr_with_port_on_network( + constants.IPV6_SLAAC, insert_address_allocated=True) + def test_create_subnet_ipv6_slaac_with_router_intf_on_network(self): self._test_create_subnet_ipv6_auto_addr_with_port_on_network( constants.IPV6_SLAAC,