Fixes regression in release_dhcp

 * regression from c96e75d6804d016da7c6356bf593eb86dcb2f257
 * fixes out of order update and retrieval of vif_id
 * includes failing test
 * fixes bug 973442

Change-Id: I3bea1c754042ad5960f285fbcdc1d45445079f81

2 files changed, 43 insertions, 5 deletions

diff --git a/nova/network/manager.py b/nova/network/manager.py
index ffecf89b0..8b89e268e 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -1200,10 +1200,11 @@ class NetworkManager(manager.SchedulerDependentManager):
 
     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})
-        fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address)
         instance_id = fixed_ip_ref['instance_id']
         self._do_trigger_security_group_members_refresh_for_instance(
                                                                    instance_id)
@@ -1219,8 +1220,6 @@ class NetworkManager(manager.SchedulerDependentManager):
 
         if FLAGS.force_dhcp_release:
             dev = self.driver.get_dev(network)
-            vif_id = fixed_ip_ref['virtual_interface_id']
-
             # NOTE(vish): The below errors should never happen, but there may
             #             be a race condition that is causing them per
             #             https://code.launchpad.net/bugs/968457, so we log
diff --git a/nova/tests/network/test_manager.py b/nova/tests/network/test_manager.py
index 7ef5a9d33..50d49b132 100644
--- a/nova/tests/network/test_manager.py
+++ b/nova/tests/network/test_manager.py
@@ -29,6 +29,7 @@ import nova.policy
 from nova import rpc
 from nova import test
 from nova import utils
+from nova.network import linux_net
 from nova.network import manager as network_manager
 from nova.tests import fake_network
 
@@ -873,6 +874,41 @@ class VlanNetworkTestCase(test.TestCase):
         db.floating_ip_destroy(context1.elevated(), float_addr)
         db.fixed_ip_disassociate(context1.elevated(), fix_addr)
 
+    def test_deallocate_fixed(self):
+        """Verify that release is called properly.
+
+        Ensures https://bugs.launchpad.net/nova/+bug/973442 doesn't return"""
+
+        def network_get(_context, network_id):
+            return networks[network_id]
+
+        self.stubs.Set(db, 'network_get', network_get)
+
+        def vif_get(_context, _vif_id):
+            return {'address': 'fake_mac'}
+
+        self.stubs.Set(db, 'virtual_interface_get', vif_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['id'])
+        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'])
+
+        self.flags(force_dhcp_release=True)
+        self.mox.StubOutWithMock(linux_net, 'release_dhcp')
+        linux_net.release_dhcp(network['bridge'], fixed['address'], 'fake_mac')
+        self.mox.ReplayAll()
+        self.network.deallocate_fixed_ip(context1, fix_addr, 'fake')
+        fixed = db.fixed_ip_get_by_address(elevated, fix_addr)
+        self.assertFalse(fixed['allocated'])
+
     def test_deallocate_fixed_no_vif(self):
         """Verify that deallocate doesn't raise when no vif is returned.
 
@@ -892,8 +928,11 @@ class VlanNetworkTestCase(test.TestCase):
         instance = db.instance_create(context1,
                 {'project_id': 'project1'})
 
-        fix_addr = db.fixed_ip_associate_pool(context1.elevated(),
-                1, instance['id'])
+        elevated = context1.elevated()
+        fix_addr = db.fixed_ip_associate_pool(elevated, 1, instance['id'])
+        values = {'allocated': True,
+                 'virtual_interface_id': 3}
+        db.fixed_ip_update(elevated, fix_addr, values)
 
         self.flags(force_dhcp_release=True)
         self.network.deallocate_fixed_ip(context1, fix_addr, 'fake')