From: Aaron Rosen <arosen@nicira.com>
Date: Tue, 8 Oct 2013 19:24:21 +0000 (-0700)
Subject: Fix dhcp_release lease race condition
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=ba99783a80ee0f92135dcfd562f3cfc9dad46d86;p=openstack-build%2Fneutron-build.git

Fix dhcp_release lease race condition

There is a possible race condition when delete or updating fixed_ips
on ports where an instance could renew its ip address again after
dhcp_release has already been executed. To fix this, the order of
reload_allocation and release_lease need to be switched. This way an
instance will not be able to renew it's ip address after it is
removed from the host file.

Fixes bug: 1237028

Change-Id: If05ec2be507378c634f5c1856dab0fbd396f43cc
---

diff --git a/neutron/agent/dhcp_agent.py b/neutron/agent/dhcp_agent.py
index b76e0f0f8..4fd9f3509 100644
--- a/neutron/agent/dhcp_agent.py
+++ b/neutron/agent/dhcp_agent.py
@@ -227,20 +227,19 @@ class DhcpAgent(manager.Manager):
         else:
             self.disable_dhcp_helper(network.id)
 
-    def release_lease_for_removed_ips(self, port, network):
+    def release_lease_for_removed_ips(self, prev_port, updated_port, network):
         """Releases the dhcp lease for ips removed from a port."""
-        prev_port = self.cache.get_port_by_id(port.id)
         if prev_port:
             previous_ips = set(fixed_ip.ip_address
                                for fixed_ip in prev_port.fixed_ips)
             current_ips = set(fixed_ip.ip_address
-                              for fixed_ip in port.fixed_ips)
+                              for fixed_ip in updated_port.fixed_ips)
             # pass in port with removed ips on it
             removed_ips = previous_ips - current_ips
             if removed_ips:
                 self.call_driver('release_lease',
                                  network,
-                                 mac_address=port.mac_address,
+                                 mac_address=updated_port.mac_address,
                                  removed_ips=removed_ips)
 
     @utils.synchronized('dhcp-agent')
@@ -283,12 +282,14 @@ class DhcpAgent(manager.Manager):
     @utils.synchronized('dhcp-agent')
     def port_update_end(self, context, payload):
         """Handle the port.update.end notification event."""
-        port = dhcp.DictModel(payload['port'])
-        network = self.cache.get_network_by_id(port.network_id)
+        updated_port = dhcp.DictModel(payload['port'])
+        network = self.cache.get_network_by_id(updated_port.network_id)
         if network:
-            self.release_lease_for_removed_ips(port, network)
-            self.cache.put_port(port)
+            prev_port = self.cache.get_port_by_id(updated_port.id)
+            self.cache.put_port(updated_port)
             self.call_driver('reload_allocations', network)
+            self.release_lease_for_removed_ips(prev_port, updated_port,
+                                               network)
 
     # Use the update handler for the port create event.
     port_create_end = port_update_end
@@ -300,13 +301,13 @@ class DhcpAgent(manager.Manager):
         if port:
             network = self.cache.get_network_by_id(port.network_id)
             self.cache.remove_port(port)
+            self.call_driver('reload_allocations', network)
             removed_ips = [fixed_ip.ip_address
                            for fixed_ip in port.fixed_ips]
             self.call_driver('release_lease',
                              network,
                              mac_address=port.mac_address,
                              removed_ips=removed_ips)
-            self.call_driver('reload_allocations', network)
 
     def enable_isolated_metadata_proxy(self, network):
 
diff --git a/neutron/tests/unit/test_dhcp_agent.py b/neutron/tests/unit/test_dhcp_agent.py
index eab75f1a4..4cc6b2537 100644
--- a/neutron/tests/unit/test_dhcp_agent.py
+++ b/neutron/tests/unit/test_dhcp_agent.py
@@ -749,12 +749,13 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
              mock.call.get_port_by_id(fake_port1.id),
              mock.call.put_port(mock.ANY)])
         self.call_driver.assert_has_calls(
-            [mock.call.call_driver(
-                'release_lease',
-                fake_network,
-                mac_address=fake_port1.mac_address,
-                removed_ips=set([updated_fake_port1.fixed_ips[0].ip_address])),
-             mock.call.call_driver('reload_allocations', fake_network)])
+            [mock.call.call_driver('reload_allocations', fake_network),
+             mock.call.call_driver(
+                 'release_lease',
+                 fake_network,
+                 mac_address=fake_port1.mac_address,
+                 removed_ips=set([updated_fake_port1.fixed_ips[0].ip_address]))
+             ])
 
     def test_port_delete_end(self):
         payload = dict(port_id=fake_port2.id)
@@ -769,11 +770,11 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
              mock.call.get_network_by_id(fake_network.id),
              mock.call.remove_port(fake_port2)])
         self.call_driver.assert_has_calls(
-            [mock.call.call_driver('release_lease',
+            [mock.call.call_driver('reload_allocations', fake_network),
+             mock.call.call_driver('release_lease',
                                    fake_network,
                                    mac_address=fake_port2.mac_address,
-                                   removed_ips=removed_ips),
-             mock.call.call_driver('reload_allocations', fake_network)])
+                                   removed_ips=removed_ips)])
 
     def test_port_delete_end_unknown_port(self):
         payload = dict(port_id='unknown')