From 44132acbe91092de1188a3015a2c7155b5ec774c Mon Sep 17 00:00:00 2001
From: David McNally <dave.mcnally@hp.com>
Date: Fri, 27 Jul 2012 13:32:14 +0100
Subject: 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            |  7 ++++---
 nova/tests/network/test_manager.py | 29 +++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 3 deletions(-)

(limited to 'nova')

diff --git a/nova/network/manager.py b/nova/network/manager.py
index 4dd42e153..9c70b3ce3 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -1273,9 +1273,6 @@ class NetworkManager(manager.SchedulerDependentManager):
         """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 @@ class NetworkManager(manager.SchedulerDependentManager):
             #             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)
diff --git a/nova/tests/network/test_manager.py b/nova/tests/network/test_manager.py
index a183094af..d04df5f13 100644
--- a/nova/tests/network/test_manager.py
+++ b/nova/tests/network/test_manager.py
@@ -1013,6 +1013,35 @@ class VlanNetworkTestCase(test.TestCase):
         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):
 
-- 
cgit