From: John Davidge <jodavidg@cisco.com>
Date: Tue, 11 Aug 2015 12:55:51 +0000 (+0100)
Subject: Fix _update_subnet_allocation_pools returning empty list
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=0b0aa4a61dce3af9e6e2d773434ced0298d60275;p=openstack-build%2Fneutron-build.git

Fix _update_subnet_allocation_pools returning empty list

_update_subnet_allocation_pools was returning an empty list in all
cases due to trying to iterate over the same generator twice.
Generators cannot be iterated over multiple times.

This patch changes the generator into a list to fix this problem,
and alters the unit test so that this issue is exposed.

Change-Id: Iea98f3ae4f16964cd68154ac5edfeb125de889e0
Closes-Bug: 1483687
---

diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py
index 43ef98002..d6adf01fb 100644
--- a/neutron/db/ipam_backend_mixin.py
+++ b/neutron/db/ipam_backend_mixin.py
@@ -158,9 +158,9 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon):
     def _update_subnet_allocation_pools(self, context, subnet_id, s):
         context.session.query(models_v2.IPAllocationPool).filter_by(
             subnet_id=subnet_id).delete()
-        pools = ((netaddr.IPAddress(p.first, p.version).format(),
+        pools = [(netaddr.IPAddress(p.first, p.version).format(),
                   netaddr.IPAddress(p.last, p.version).format())
-                 for p in s['allocation_pools'])
+                 for p in s['allocation_pools']]
         new_pools = [models_v2.IPAllocationPool(first_ip=p[0],
                                                 last_ip=p[1],
                                                 subnet_id=subnet_id)
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 8bdb54bbd..4024b8d94 100644
--- a/neutron/tests/unit/db/test_db_base_plugin_v2.py
+++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py
@@ -4167,6 +4167,19 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
                 self.assertEqual(res.status_int,
                                  webob.exc.HTTPClientError.code)
 
+    def _verify_updated_subnet_allocation_pools(self, res, with_gateway_ip):
+        res = self.deserialize(self.fmt, res)
+        self.assertEqual(len(res['subnet']['allocation_pools']), 2)
+        res_vals = (
+            list(res['subnet']['allocation_pools'][0].values()) +
+            list(res['subnet']['allocation_pools'][1].values())
+        )
+        for pool_val in ['10', '20', '30', '40']:
+            self.assertTrue('192.168.0.%s' % (pool_val) in res_vals)
+        if with_gateway_ip:
+            self.assertEqual((res['subnet']['gateway_ip']),
+                             '192.168.0.9')
+
     def _test_update_subnet_allocation_pools(self, with_gateway_ip=False):
         """Test that we can successfully update with sane params.
 
@@ -4187,22 +4200,17 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
                     data['subnet']['gateway_ip'] = '192.168.0.9'
                 req = self.new_update_request('subnets', data,
                                               subnet['subnet']['id'])
-                #check res code but then do GET on subnet for verification
+                #check res code and contents
                 res = req.get_response(self.api)
                 self.assertEqual(res.status_code, 200)
+                self._verify_updated_subnet_allocation_pools(res,
+                                                             with_gateway_ip)
+                #GET subnet to verify DB updated correctly
                 req = self.new_show_request('subnets', subnet['subnet']['id'],
                                             self.fmt)
-                res = self.deserialize(self.fmt, req.get_response(self.api))
-                self.assertEqual(len(res['subnet']['allocation_pools']), 2)
-                res_vals = (
-                    list(res['subnet']['allocation_pools'][0].values()) +
-                    list(res['subnet']['allocation_pools'][1].values())
-                )
-                for pool_val in ['10', '20', '30', '40']:
-                    self.assertTrue('192.168.0.%s' % (pool_val) in res_vals)
-                if with_gateway_ip:
-                    self.assertEqual((res['subnet']['gateway_ip']),
-                                     '192.168.0.9')
+                res = req.get_response(self.api)
+                self._verify_updated_subnet_allocation_pools(res,
+                                                             with_gateway_ip)
 
     def test_update_subnet_allocation_pools(self):
         self._test_update_subnet_allocation_pools()