From 0109578a8ec07f743f7e2b654007e17f145ea20f Mon Sep 17 00:00:00 2001 From: Eugene Nikanorov Date: Sat, 18 Apr 2015 15:31:44 +0400 Subject: [PATCH] Fix incorrect query for user ip allocations Previously the query was fetching an IPAllocation object incorrectly relying on the fact that it has port attribute that should be join-loaded when it really is not. Incorrect query produced by previous code: SELECT ipallocations.port_id AS ipallocations_port_id, ipallocations.ip_address AS ipallocations_ip_address, ipallocations.subnet_id AS ipallocations_subnet_id, ipallocations.network_id AS ipallocations_network_id FROM ipallocations, ports WHERE ipallocations.subnet_id = :subnet_id_1 AND ports.device_owner NOT IN (:device_owner_1) The query then may have produced results that don't satisfy the condition intended by the code. Query produced by the fixed code: SELECT ipallocations.port_id AS ipallocations_port_id, ipallocations.ip_address AS ipallocations_ip_address, ipallocations.subnet_id AS ipallocations_subnet_id, ipallocations.network_id AS ipallocations_network_id FROM ipallocations JOIN ports ON ports.id = ipallocations.port_id WHERE ipallocations.subnet_id = :subnet_id_1 AND ports.device_owner NOT IN (:device_owner_1) Change-Id: I34682df784e30e3ce49ee48c690f8b799ad58149 Closes-Bug: #1357055 --- neutron/db/db_base_plugin_v2.py | 5 +++- .../tests/unit/db/test_db_base_plugin_v2.py | 23 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index b801fd57a38..7a171816e87 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -1514,8 +1514,11 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, def _subnet_get_user_allocation(self, context, subnet_id): """Check if there are any user ports on subnet and return first.""" + # need to join with ports table as IPAllocation's port + # is not joined eagerly and thus producing query which yields + # incorrect results return (context.session.query(models_v2.IPAllocation). - filter_by(subnet_id=subnet_id).join(). + filter_by(subnet_id=subnet_id).join(models_v2.Port). filter(~models_v2.Port.device_owner. in_(AUTO_DELETE_PORT_OWNERS)).first()) 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 e8ef97e8fdb..044b43e377b 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -2730,6 +2730,29 @@ class TestNetworksV2(NeutronDbPluginV2TestCase): res = self.deserialize(self.fmt, req) self.assertEqual(res['network']['admin_state_up'], v[1]) + def test_get_user_allocation_for_dhcp_port_returns_none(self): + plugin = manager.NeutronManager.get_plugin() + if not hasattr(plugin, '_subnet_get_user_allocation'): + return + with contextlib.nested( + self.network(), + self.network() + ) as (net, net1): + with contextlib.nested( + self.subnet(network=net, cidr='10.0.0.0/24'), + self.subnet(network=net1, cidr='10.0.1.0/24') + ) as (subnet, subnet1): + with contextlib.nested( + self.port(subnet=subnet, device_owner='network:dhcp'), + self.port(subnet=subnet1) + ) as (p, p2): + # check that user allocations on another network don't + # affect _subnet_get_user_allocation method + res = plugin._subnet_get_user_allocation( + context.get_admin_context(), + subnet['subnet']['id']) + self.assertIsNone(res) + class TestSubnetsV2(NeutronDbPluginV2TestCase):