improved network error case handling for fixed ips

3 files changed, 65 insertions, 13 deletions

diff --git a/bin/nova-dhcpbridge b/bin/nova-dhcpbridge
index 42eaf4bcb..2f75bf43b 100755
--- a/bin/nova-dhcpbridge
+++ b/bin/nova-dhcpbridge
@@ -46,16 +46,17 @@ flags.DECLARE('network_size', 'nova.network.manager')
 flags.DECLARE('num_networks', 'nova.network.manager')
 
 
-def add_lease(_mac, ip_address, _hostname, _interface):
+def add_lease(mac, ip_address, _hostname, _interface):
     """Set the IP that was assigned by the DHCP server."""
     if FLAGS.fake_rabbit:
         logging.debug("leasing ip")
         network_manager = utils.import_object(FLAGS.network_manager)
-        network_manager.lease_fixed_ip(None, ip_address)
+        network_manager.lease_fixed_ip(None, mac, ip_address)
     else:
         rpc.cast("%s.%s" % (FLAGS.network_topic, FLAGS.host),
                  {"method": "lease_fixed_ip",
                   "args": {"context": None,
+                           "mac": mac,
                            "address": ip_address}})
 
 
@@ -64,16 +65,17 @@ def old_lease(_mac, _ip_address, _hostname, _interface):
     logging.debug("Adopted old lease or got a change of mac/hostname")
 
 
-def del_lease(_mac, ip_address, _hostname, _interface):
+def del_lease(mac, ip_address, _hostname, _interface):
     """Called when a lease expires."""
     if FLAGS.fake_rabbit:
         logging.debug("releasing ip")
         network_manager = utils.import_object(FLAGS.network_manager)
-        network_manager.release_fixed_ip(None, ip_address)
+        network_manager.release_fixed_ip(None, mac, ip_address)
     else:
         rpc.cast("%s.%s" % (FLAGS.network_topic, FLAGS.host),
                  {"method": "release_fixed_ip",
                   "args": {"context": None,
+                           "mac": mac,
                            "address": ip_address}})
 
 
diff --git a/nova/network/manager.py b/nova/network/manager.py
index 3212a7eab..79280384c 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -226,19 +226,42 @@ class VlanManager(NetworkManager):
                                             network_ref['vpn_private_address'])
         self.driver.update_dhcp(context, network_ref['id'])
 
-    def lease_fixed_ip(self, context, address):
+    def lease_fixed_ip(self, context, mac, address):
         """Called by dhcp-bridge when ip is leased"""
         logging.debug("Leasing IP %s", address)
         fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address)
         if not fixed_ip_ref['allocated']:
             logging.warn("IP %s leased that was already deallocated", address)
+            return
+        instance_ref = self.db.fixed_ip_get_instance(context, address)
+        if not instance_ref:
+            raise exception.Error("IP %s leased that isn't associated" %
+                                  address)
+        if instance_ref['mac_address'] != mac:
+            raise exception.Error("IP %s leased to bad mac %s vs %s" %
+                                  (address, instance_ref['mac_address'], mac))
         self.db.fixed_ip_update(context,
                                 fixed_ip_ref['str_id'],
                                 {'leased': True})
 
-    def release_fixed_ip(self, context, address):
+    def release_fixed_ip(self, context, mac, address):
         """Called by dhcp-bridge when ip is released"""
         logging.debug("Releasing IP %s", address)
+        fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address)
+        if not fixed_ip_ref['leased']:
+            logging.warn("IP %s released that was not leased", address)
+            return
+        instance_ref = self.db.fixed_ip_get_instance(context, address)
+        if not instance_ref:
+            raise exception.Error("IP %s released that isn't associated" %
+                                  address)
+        if instance_ref['mac_address'] != mac:
+            raise exception.Error("IP %s released from bad mac %s vs %s" %
+                                  (address, instance_ref['mac_address'], mac))
+        if fixed_ip_ref['allocated']:
+            logging.warn("IP %s released that is still allocated", address)
+            self.db.fixed_ip_update(context, address, {'leased': False})
+            return
         self.db.fixed_ip_update(context, address, {'allocated': False,
                                                    'leased': False})
         self.db.fixed_ip_instance_disassociate(context, address)
diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py
index 9958600e0..d8d4ec0c3 100644
--- a/nova/tests/network_unittest.py
+++ b/nova/tests/network_unittest.py
@@ -147,10 +147,23 @@ class NetworkTestCase(test.TrialTestCase):
         """Makes sure that private ips don't overlap"""
         first = self._create_address(0)
         lease_ip(first)
+        instance_ids = []
         for i in range(1, 5):
-            address = self._create_address(i)
-            address2 = self._create_address(i)
-            address3 = self._create_address(i)
+            mac = utils.generate_mac()
+            instance_ref = db.instance_create(None,
+                                              {'mac_address': mac})
+            instance_ids.append(instance_ref['id'])
+            address = self._create_address(i, instance_ref['id'])
+            mac = utils.generate_mac()
+            instance_ref = db.instance_create(None,
+                                              {'mac_address': mac})
+            instance_ids.append(instance_ref['id'])
+            address2 = self._create_address(i, instance_ref['id'])
+            mac = utils.generate_mac()
+            instance_ref = db.instance_create(None,
+                                              {'mac_address': mac})
+            instance_ids.append(instance_ref['id'])
+            address3 = self._create_address(i, instance_ref['id'])
             lease_ip(address)
             lease_ip(address2)
             lease_ip(address3)
@@ -166,6 +179,8 @@ class NetworkTestCase(test.TrialTestCase):
             release_ip(address)
             release_ip(address2)
             release_ip(address3)
+        for instance_id in instance_ids:
+            db.instance_destroy(None, instance_id)
         release_ip(first)
         db.fixed_ip_deallocate(None, first)
 
@@ -226,8 +241,13 @@ class NetworkTestCase(test.TrialTestCase):
         num_available_ips = db.network_count_available_ips(None,
                                                            network['id'])
         addresses = []
+        instance_ids = []
         for i in range(num_available_ips):
-            address = self._create_address(0)
+            mac = utils.generate_mac()
+            instance_ref = db.instance_create(None,
+                                              {'mac_address': mac})
+            instance_ids.append(instance_ref['id'])
+            address = self._create_address(0, instance_ref['id'])
             addresses.append(address)
             lease_ip(address)
 
@@ -238,9 +258,10 @@ class NetworkTestCase(test.TrialTestCase):
                           None,
                           network['id'])
 
-        for i in range(len(addresses)):
+        for i in range(num_available_ips):
             db.fixed_ip_deallocate(None, addresses[i])
             release_ip(addresses[i])
+            db.instance_destroy(None, instance_ids[i])
         self.assertEqual(db.network_count_available_ips(None,
                                                         network['id']),
                          num_available_ips)
@@ -263,7 +284,10 @@ def binpath(script):
 def lease_ip(private_ip):
     """Run add command on dhcpbridge"""
     network_ref = db.fixed_ip_get_network(None, private_ip)
-    cmd = "%s add fake %s fake" % (binpath('nova-dhcpbridge'), private_ip)
+    instance_ref = db.fixed_ip_get_instance(None, private_ip)
+    cmd = "%s add %s %s fake" % (binpath('nova-dhcpbridge'),
+                                 instance_ref['mac_address'],
+                                 private_ip)
     env = {'DNSMASQ_INTERFACE': network_ref['bridge'],
            'TESTING': '1',
            'FLAGFILE': FLAGS.dhcpbridge_flagfile}
@@ -274,7 +298,10 @@ def lease_ip(private_ip):
 def release_ip(private_ip):
     """Run del command on dhcpbridge"""
     network_ref = db.fixed_ip_get_network(None, private_ip)
-    cmd = "%s del fake %s fake" % (binpath('nova-dhcpbridge'), private_ip)
+    instance_ref = db.fixed_ip_get_instance(None, private_ip)
+    cmd = "%s del %s %s fake" % (binpath('nova-dhcpbridge'),
+                                 instance_ref['mac_address'],
+                                 private_ip)
     env = {'DNSMASQ_INTERFACE': network_ref['bridge'],
            'TESTING': '1',
            'FLAGFILE': FLAGS.dhcpbridge_flagfile}