From 1254f2371864e10dc66f5aa27031088209f74e9e 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 Conflicts: neutron/ipam/exceptions.py Change-Id: I5b96b3a91aacffe118ddbb91a75c4892818ba97a (cherry picked from commit 83b6ce9e9ec2f0261fed630e18cbe9cfa363a0dd) (cherry picked from commit 2a6a4ab5dacc2e92c24d68774bf286c9e6ae2d94) --- neutron/db/db_base_plugin_v2.py | 7 ------- neutron/ipam/drivers/neutrondb_ipam/driver.py | 10 ++-------- neutron/ipam/exceptions.py | 2 ++ neutron/tests/unit/db/test_db_base_plugin_v2.py | 16 ---------------- .../ipam/drivers/neutrondb_ipam/test_driver.py | 6 ------ 5 files changed, 4 insertions(+), 37 deletions(-) diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 230294e4313..b77a2f70f96 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -1507,13 +1507,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 6969ef564a3..9123149bb51 100644 --- a/neutron/ipam/drivers/neutrondb_ipam/driver.py +++ b/neutron/ipam/drivers/neutrondb_ipam/driver.py @@ -287,14 +287,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..1bbc7a47bd8 100644 --- a/neutron/ipam/exceptions.py +++ b/neutron/ipam/exceptions.py @@ -31,6 +31,8 @@ class InvalidAddressType(exceptions.NeutronException): 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): message = _("Unable to find IP address %(ip_address)s on 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 51faa21a907..07c3f5995f2 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -1427,22 +1427,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 79c90fe7153..cb584bcb346 100644 --- a/neutron/tests/unit/ipam/drivers/neutrondb_ipam/test_driver.py +++ b/neutron/tests/unit/ipam/drivers/neutrondb_ipam/test_driver.py @@ -422,12 +422,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