Moving where the fixed ip deallocation happens.

Fixes bug 1021352.

In network/manager/deallocate_fixed_ip the call
to mark the IP as no longer allocated occurs before
the call to update security group rules. This means
that if an error occurs in the security group
processing, or if for some reason it is very slow
there is a risk that that the address is reused by
another tenant before the rules relating to this address
have been fully revoked.

Moving the db call to after the call to trigger the
security group refresh would avoid the issue when an
error occurs (the fixed_ip should not be released in
this case).

Change-Id: Iaba1af5c9a17fbbb82e42522b1060773de61550a

nova/network/manager.py

@@ -1273,9 +1273,6 @@ def deallocate_fixed_ip(self, context, address, **kwargs):
         """Returns a fixed ip to the pool."""
         fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address)
         vif_id = fixed_ip_ref['virtual_interface_id']
-        self.db.fixed_ip_update(context, address,
-                                {'allocated': False,
-                                 'virtual_interface_id': None})
         instance = self.db.instance_get_by_uuid(context,
                                                 fixed_ip_ref['instance_uuid'])
 
@@ -1312,6 +1309,10 @@ def deallocate_fixed_ip(self, context, address, **kwargs):
             #             callback will get called by nova-dhcpbridge.
             self.driver.release_dhcp(dev, address, vif['address'])
 
+        self.db.fixed_ip_update(context, address,
+                                {'allocated': False,
+                                 'virtual_interface_id': None})
+
     def lease_fixed_ip(self, context, address):
         """Called by dhcp-bridge when ip is leased."""
         LOG.debug(_('Leased IP |%(address)s|'), locals(), context=context)

nova/tests/network/test_manager.py

@@ -1013,6 +1013,35 @@ def vif_get(_context, _vif_id):
         self.flags(force_dhcp_release=True)
         self.network.deallocate_fixed_ip(context1, fix_addr, 'fake')
 
+    def test_fixed_ip_cleanup_fail(self):
+        """Verify IP is not deallocated if the security group refresh fails."""
+        def network_get(_context, network_id):
+            return networks[network_id]
+
+        self.stubs.Set(db, 'network_get', network_get)
+
+        context1 = context.RequestContext('user', 'project1')
+
+        instance = db.instance_create(context1,
+                {'project_id': 'project1'})
+
+        elevated = context1.elevated()
+        fix_addr = db.fixed_ip_associate_pool(elevated, 1, instance['uuid'])
+        values = {'allocated': True,
+                  'virtual_interface_id': 3}
+        db.fixed_ip_update(elevated, fix_addr, values)
+        fixed = db.fixed_ip_get_by_address(elevated, fix_addr)
+        network = db.network_get(elevated, fixed['network_id'])
+
+        db.instance_destroy(self.context.elevated(), instance['uuid'])
+        self.assertRaises(exception.InstanceNotFound,
+                                  self.network.deallocate_fixed_ip,
+                                  context1,
+                                  fix_addr,
+                                  'fake')
+        fixed = db.fixed_ip_get_by_address(elevated, fix_addr)
+        self.assertTrue(fixed['allocated'])
+
 
 class CommonNetworkTestCase(test.TestCase):