Remove race condition (in FloatingIps)

Soft delete all duplicate rows with the same address except one with
the biggest value in `id` column.

Create UC on columns (address, deleted)

Fix floating_ip_create
  Replace Select then Insert -> Try to Insert.

Fix floating_ip_bulk_create
  Replace get all and locally compare -> Try to Insert

Fix floating_ip_update
  If we update floating_ip with existing address, raise
  rollback and raise FloatingIpExists

Add missing test for floating_ip_update to duplicate

Fixes bug 1145216

blueprint db-enforce-unique-keys

Change-Id: Idd0c8305ca348700235f757a29fc7998edca576b

2 files changed, 39 insertions, 0 deletions

diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py
index fcba2aefa..a3c281b49 100644
--- a/nova/tests/test_db_api.py
+++ b/nova/tests/test_db_api.py
@@ -2989,6 +2989,15 @@ class FloatingIpTestCase(test.TestCase, ModelsObjectComparatorMixin):
                                                'deleted', 'fixed_ip_id',
                                                'fixed_ip'])
 
+    def test_floating_ip_update_to_duplicate(self):
+        float_ip1 = self._create_floating_ip({'address': '1.1.1.1'})
+        float_ip2 = self._create_floating_ip({'address': '1.1.1.2'})
+
+        self.assertRaises(exception.FloatingIpExists,
+                          db.floating_ip_update,
+                          self.ctxt, float_ip2['address'],
+                          {'address': float_ip1['address']})
+
 
 class InstanceDestroyConstraints(test.TestCase):
 
diff --git a/nova/tests/test_migrations.py b/nova/tests/test_migrations.py
index 8c2f04f21..a0f71b25a 100644
--- a/nova/tests/test_migrations.py
+++ b/nova/tests/test_migrations.py
@@ -1285,6 +1285,36 @@ class TestNovaMigrations(BaseMigrationTestCase, CommonTestsMixIn):
 
         self.assertFalse('availability_zone' in rows[0])
 
+    def _pre_upgrade_177(self, engine):
+        floating_ips = get_table(engine, 'floating_ips')
+        data = [
+            {'address': '128.128.128.128', 'deleted': 0},
+            {'address': '128.128.128.128', 'deleted': 0},
+            {'address': '128.128.128.129', 'deleted': 0},
+        ]
+
+        for item in data:
+            floating_ips.insert().values(item).execute()
+        return data
+
+    def _check_177(self, engine, data):
+        floating_ips = get_table(engine, 'floating_ips')
+
+        def get_(address, deleted):
+            deleted_value = 0 if not deleted else floating_ips.c.id
+            return floating_ips.select().\
+                        where(floating_ips.c.address == address).\
+                        where(floating_ips.c.deleted == deleted_value).\
+                        execute().\
+                        fetchall()
+
+        self.assertEqual(1, len(get_('128.128.128.128', False)))
+        self.assertEqual(1, len(get_('128.128.128.128', True)))
+        self.assertEqual(1, len(get_('128.128.128.129', False)))
+        self.assertRaises(sqlalchemy.exc.IntegrityError,
+                          floating_ips.insert().execute,
+                          dict(address='128.128.128.129', deleted=0))
+
 
 class TestBaremetalMigrations(BaseMigrationTestCase, CommonTestsMixIn):
     """Test sqlalchemy-migrate migrations."""