cyclades: Do not release floating IPs

Update 'release_instance_nics' backend function which is called to
process NIC modifications in the Ganeti backend, to not return a
IPv4 address back to pool if this address is a floating IP.

snf-cyclades-app/synnefo/api/floating_ips.py

@@ -144,6 +144,7 @@ def allocate_floating_ip(request):
             address = util.get_network_free_address(network)  # Get X-Lock
         else:
             if FloatingIP.objects.filter(network=network,
+                                         deleted=False,
                                          ipv4=address).exists():
                 msg = "Floating IP '%s' is reserved" % address
                 raise faults.Conflict(msg)
@@ -190,6 +191,9 @@ def release_floating_ip(request, floating_ip_id):
         raise faults.ItemNotFound("Floating IP '%s' does not exist" %
                                   floating_ip_id)
 
+    # Since we have got an exlusively lock in floating IP, and since
+    # to remove a floating IP you need the same lock, the in_use() query
+    # is safe
     if floating_ip.in_use():
         msg = "Floating IP '%s' is used" % floating_ip.id
         raise faults.Conflict(message=msg)

snf-cyclades-app/synnefo/logic/backend.py

@@ -35,6 +35,7 @@ from django.db import transaction
 from datetime import datetime
 
 from synnefo.db.models import (Backend, VirtualMachine, Network,
+                               FloatingIP,
                                BackendNetwork, BACKEND_STATUSES,
                                pooled_rapi_client, VirtualMachineDiagnostic,
                                Flavor)
@@ -160,7 +161,10 @@ def process_op_status(vm, etime, jobid, opcode, status, logmsg, nics=None,
         # See ticket #799 for all the details.
         if status == 'success' or (status == 'error' and
                                    vm.operstate == 'ERROR'):
-            _process_net_status(vm, etime, nics=[])
+            # VM has been deleted. Release the instance IPs
+            release_instance_ips(vm, [])
+            # And delete the releated NICs (must be performed after release!)
+            vm.nics.all().delete()
             vm.deleted = True
             vm.operstate = state_for_success
             vm.backendtime = etime
@@ -220,7 +224,10 @@ def _process_net_status(vm, etime, nics):
     # guarantee that no deadlock will occur with Backend allocator.
     Backend.objects.select_for_update().get(id=vm.backend_id)
 
-    release_instance_nics(vm)
+    # NICs have changed. Release the instance IPs
+    release_instance_ips(vm, ganeti_nics)
+    # And delete the releated NICs (must be performed after release!)
+    vm.nics.all().delete()
 
     for nic in ganeti_nics:
         ipv4 = nic.get('ipv4', '')
@@ -286,13 +293,29 @@ def nics_changed(old_nics, new_nics):
     return False
 
 
-def release_instance_nics(vm):
-    for nic in vm.nics.all():
-        net = nic.network
-        if nic.ipv4:
-            net.release_address(nic.ipv4)
-        nic.delete()
-        net.save()
+def release_instance_ips(vm, ganeti_nics):
+    old_addresses = set(vm.nics.values_list("network", "ipv4"))
+    new_addresses = set(map(lambda nic: (nic["network"], nic["ipv4"]),
+                            ganeti_nics))
+    to_release = old_addresses - new_addresses
+    for (network_id, ipv4) in to_release:
+        if ipv4:
+            net = Network.objects.get(id=network_id)
+            # Important: Take exclusive lock in pool before checking if there
+            # is a floating IP with this ipv4 address, otherwise there is a
+            # race condition, where you may release a floating IP that has been
+            # created after search floating IPs and before you get exclusively
+            # the pool
+            pool = net.get_pool()
+            try:
+                floating_ip = net.floating_ips.select_for_update()\
+                                              .get(ipv4=ipv4, machine=vm,
+                                                   deleted=False)
+                floating_ip.machine = None
+                floating_ip.save()
+            except FloatingIP.DoesNotExist:
+                net.release_address(ipv4)
+                pool.save()
 
 
 @transaction.commit_on_success

snf-cyclades-app/synnefo/logic/tests.py

@@ -254,7 +254,8 @@ class UpdateDBTest(TestCase):
     def test_remove(self, client):
         vm = mfactory.VirtualMachineFactory()
         # Also create a NIC
-        mfactory.NetworkInterfaceFactory(machine=vm)
+        nic = mfactory.NetworkInterfaceFactory(machine=vm)
+        nic.network.get_pool().reserve(nic.ipv4)
         msg = self.create_msg(operation='OP_INSTANCE_REMOVE',
                               instance=vm.backend_vm_id)
         with mocked_quotaholder():
@@ -265,6 +266,33 @@ class UpdateDBTest(TestCase):
         self.assertTrue(db_vm.deleted)
         # Check that nics are deleted
         self.assertFalse(db_vm.nics.all())
+        self.assertTrue(nic.network.get_pool().is_available(nic.ipv4))
+        vm2 = mfactory.VirtualMachineFactory()
+        network = mfactory.NetworkFactory()
+        fp1 = mfactory.FloatingIPFactory(machine=vm2, network=network)
+        fp2 = mfactory.FloatingIPFactory(machine=vm2, network=network)
+        mfactory.NetworkInterfaceFactory(machine=vm2, network=network,
+                ipv4=fp1.ipv4)
+        mfactory.NetworkInterfaceFactory(machine=vm2, network=network,
+                ipv4=fp2.ipv4)
+        pool = network.get_pool()
+        pool.reserve(fp1.ipv4)
+        pool.reserve(fp2.ipv4)
+        pool.save()
+        msg = self.create_msg(operation='OP_INSTANCE_REMOVE',
+                              instance=vm2.backend_vm_id)
+        with mocked_quotaholder():
+            update_db(client, msg)
+        client.basic_ack.assert_called_once()
+        db_vm = VirtualMachine.objects.get(id=vm.id)
+        self.assertEqual(db_vm.operstate, 'DESTROYED')
+        self.assertTrue(db_vm.deleted)
+        self.assertEqual(FloatingIP.objects.get(id=fp1.id).machine, None)
+        self.assertEqual(FloatingIP.objects.get(id=fp2.id).machine, None)
+        pool = network.get_pool()
+        # Test that floating ips are not released
+        self.assertFalse(pool.is_available(fp1.ipv4))
+        self.assertFalse(pool.is_available(fp2.ipv4))
 
     def test_create(self, client):
         vm = mfactory.VirtualMachineFactory()