Remove exception `IpAddressAllocationNotFound`

This patch removes the ``IpAddressAllocationNotFound`` exception. This
exception was raised when a IPAM register was called to be deleted
but not found.

As reported in the LP bug, this IPAM register deletion can be called
several times if a port fails during the creation. The IPAM register
deletion calls the DB deletion but doesn't raise any exception if the
register does not exist. The code ensures the IPAM register is
deleted and there is no need to fail if it is not present anymore.

This patch also removes the exception catch and try in "update_port",
that was added in [0] as a fix for [1]. That was added because the
subnet deletion code involved a port update call [2] during the
IP allocation deletion, if any port was still present in the subnet.
Since [3], this code is not needed because the subnet deletion does
not call a port update anymore.

[0]https://review.opendev.org/c/openstack/neutron/+/373536
[1]https://bugs.launchpad.net/neutron/+bug/1622616
[2]https://github.com/openstack/neutron/blob/pike-em/neutron/db/db_base_plugin_v2.py#L1017-L1018
[3]https://review.opendev.org/c/openstack/neutron/+/713045

Closes-Bug: #1965807
Related-Bug: #1954763
Related-Bug: #1622616

Conflicts:
    neutron/ipam/exceptions.py

Change-Id: I5b96b3a91aacffe118ddbb91a75c4892818ba97a
(cherry picked from commit 83b6ce9e9e)
This commit is contained in:
Rodolfo Alonso Hernandez 2022-03-16 16:32:31 +00:00
parent 962d075fb8
commit 2a6a4ab5da
5 changed files with 4 additions and 37 deletions

View File

@ -1524,13 +1524,6 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
old_port_db=db_port, old_port_db=db_port,
old_port=self._make_port_dict(db_port), old_port=self._make_port_dict(db_port),
new_port=new_port) new_port=new_port)
except ipam_exc.IpAddressAllocationNotFound as e:
# If a port update and a subnet delete interleave, there is a
# chance that the IPAM update operation raises this exception.
# Rather than throwing that up to the user under some sort of
# conflict, bubble up a retry instead that should bring things
# back to sanity.
raise os_db_exc.RetryRequest(e)
except ipam_exc.IPAddressChangeNotAllowed as e: except ipam_exc.IPAddressChangeNotAllowed as e:
raise exc.BadRequest(resource='ports', msg=e) raise exc.BadRequest(resource='ports', msg=e)
return self._make_port_dict(db_port) return self._make_port_dict(db_port)

View File

@ -281,14 +281,8 @@ class NeutronDbSubnet(ipam_base.Subnet):
def deallocate(self, address): def deallocate(self, address):
# This is almost a no-op because the Neutron DB IPAM driver does not # This is almost a no-op because the Neutron DB IPAM driver does not
# delete IPAllocation objects at every deallocation. The only # delete IPAllocation objects at every deallocation. The only
# operation it performs is to delete an IPRequest entry. # operation it performs is to delete an IPAMAllocation entry.
count = self.subnet_manager.delete_allocation( self.subnet_manager.delete_allocation(self._context, address)
self._context, address)
# count can hardly be greater than 1, but it can be 0...
if not count:
raise ipam_exc.IpAddressAllocationNotFound(
subnet_id=self.subnet_manager.neutron_id,
ip_address=address)
def _no_pool_changes(self, context, pools): def _no_pool_changes(self, context, pools):
"""Check if pool updates in db are required.""" """Check if pool updates in db are required."""

View File

@ -31,6 +31,8 @@ class InvalidAddressType(exceptions.NeutronException):
message = _("Unknown address type %(address_type)s") message = _("Unknown address type %(address_type)s")
# NOTE(ralonsoh): this exception is used in VMware NSX plugin but is removed
# in Zed release.
class IpAddressAllocationNotFound(exceptions.NeutronException): class IpAddressAllocationNotFound(exceptions.NeutronException):
message = _("Unable to find IP address %(ip_address)s on subnet " message = _("Unable to find IP address %(ip_address)s on subnet "
"%(subnet_id)s") "%(subnet_id)s")

View File

@ -1498,22 +1498,6 @@ fixed_ips=ip_address%%3D%s&fixed_ips=ip_address%%3D%s&fixed_ips=subnet_id%%3D%s
res = req.get_response(self.api) res = req.get_response(self.api)
self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int) self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int)
def test_port_update_with_ipam_error(self):
with self.network() as network,\
self.subnet(), self.subnet(),\
self.port(network=network) as port,\
mock.patch('neutron.ipam.drivers.neutrondb_ipam.'
'driver.NeutronDbSubnet.deallocate') as f:
f.side_effect = [
ipam_exc.IpAddressAllocationNotFound(
ip_address='foo_i', subnet_id='foo_s'),
None,
]
data = {'port': {'name': 'fool-me'}}
req = self.new_update_request('ports', data, port['port']['id'])
res = self.deserialize(self.fmt, req.get_response(self.api))
self.assertEqual('fool-me', res['port']['name'])
def test_update_port(self): def test_update_port(self):
with self.port() as port: with self.port() as port:
data = {'port': {'admin_state_up': False}} data = {'port': {'admin_state_up': False}}

View File

@ -423,12 +423,6 @@ class TestNeutronDbIpamSubnet(testlib_api.SqlTestCase,
# future proofing in case v6-specific logic will be added. # future proofing in case v6-specific logic will be added.
self._test_deallocate_address('fde3:abcd:4321:1::/64', 6) self._test_deallocate_address('fde3:abcd:4321:1::/64', 6)
def test_allocate_unallocated_address_fails(self):
ipam_subnet = self._create_and_allocate_ipam_subnet(
'10.0.0.0/24', ip_version=constants.IP_VERSION_4)[0]
self.assertRaises(ipam_exc.IpAddressAllocationNotFound,
ipam_subnet.deallocate, '10.0.0.2')
def test_allocate_all_pool_addresses_triggers_range_recalculation(self): def test_allocate_all_pool_addresses_triggers_range_recalculation(self):
# This test instead might be made to pass, but for the wrong reasons! # This test instead might be made to pass, but for the wrong reasons!
pass pass