From 83b6ce9e9ec2f0261fed630e18cbe9cfa363a0dd Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 16 Mar 2022 16:32:31 +0000 Subject: [PATCH] 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 Change-Id: I5b96b3a91aacffe118ddbb91a75c4892818ba97a --- neutron/db/db_base_plugin_v2.py | 7 ------- neutron/ipam/drivers/neutrondb_ipam/driver.py | 10 ++-------- neutron/ipam/exceptions.py | 5 ----- neutron/tests/unit/db/test_db_base_plugin_v2.py | 16 ---------------- .../ipam/drivers/neutrondb_ipam/test_driver.py | 6 ------ 5 files changed, 2 insertions(+), 42 deletions(-) diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 6364c90bd63..0b56b6cce88 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -1524,13 +1524,6 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon, old_port_db=db_port, old_port=self._make_port_dict(db_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: raise exc.BadRequest(resource='ports', msg=e) return self._make_port_dict(db_port) diff --git a/neutron/ipam/drivers/neutrondb_ipam/driver.py b/neutron/ipam/drivers/neutrondb_ipam/driver.py index a49cafcb7ca..2641ce09566 100644 --- a/neutron/ipam/drivers/neutrondb_ipam/driver.py +++ b/neutron/ipam/drivers/neutrondb_ipam/driver.py @@ -281,14 +281,8 @@ class NeutronDbSubnet(ipam_base.Subnet): def deallocate(self, address): # This is almost a no-op because the Neutron DB IPAM driver does not # delete IPAllocation objects at every deallocation. The only - # operation it performs is to delete an IPRequest entry. - count = self.subnet_manager.delete_allocation( - 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) + # operation it performs is to delete an IPAMAllocation entry. + self.subnet_manager.delete_allocation(self._context, address) def _no_pool_changes(self, context, pools): """Check if pool updates in db are required.""" diff --git a/neutron/ipam/exceptions.py b/neutron/ipam/exceptions.py index badeef82b4e..d8f351ec990 100644 --- a/neutron/ipam/exceptions.py +++ b/neutron/ipam/exceptions.py @@ -31,11 +31,6 @@ class InvalidAddressType(exceptions.NeutronException): message = _("Unknown address type %(address_type)s") -class IpAddressAllocationNotFound(exceptions.NeutronException): - message = _("Unable to find IP address %(ip_address)s on subnet " - "%(subnet_id)s") - - class IpAddressAlreadyAllocated(exceptions.Conflict): message = _("IP address %(ip)s already allocated in subnet %(subnet_id)s") 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 9478e4e24c1..07878f852d1 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -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) 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): with self.port() as port: data = {'port': {'admin_state_up': False}} diff --git a/neutron/tests/unit/ipam/drivers/neutrondb_ipam/test_driver.py b/neutron/tests/unit/ipam/drivers/neutrondb_ipam/test_driver.py index 2fe3bf35a92..e5df5e58349 100644 --- a/neutron/tests/unit/ipam/drivers/neutrondb_ipam/test_driver.py +++ b/neutron/tests/unit/ipam/drivers/neutrondb_ipam/test_driver.py @@ -423,12 +423,6 @@ class TestNeutronDbIpamSubnet(testlib_api.SqlTestCase, # future proofing in case v6-specific logic will be added. 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): # This test instead might be made to pass, but for the wrong reasons! pass