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

diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py
index b801fd57a38ce5e6c890ab3cf842685c031f1e0a..7a171816e87e3430e005ebd85b15cbc206318275 100644 (file)
--- 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 e8ef97e8fdb353e6dc107a49f152ed9e2462c57e..044b43e377b2e7978a82895ef4d0056e19660836 100644 (file)
--- 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):