Merge "Fix disassociate query to remove foreign keys" into milestone-proposed

2 files changed, 64 insertions, 10 deletions

diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index c2befbabc8..e9c34ab1fc 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -993,16 +993,20 @@ def fixed_ip_disassociate_all_by_timeout(context, host, time):
     host_filter = or_(and_(models.Instance.host == host,
                            models.Network.multi_host == True),
                       models.Network.host == host)
-    fixed_ips = model_query(context, models.FixedIp, session=session,
-                       read_deleted="yes").\
-                      filter(models.FixedIp.updated_at < time).\
-                      filter(models.FixedIp.instance_id != None).\
-                      filter(models.FixedIp.allocated == False).\
-                      filter(host_filter).\
-                      all()
-    fixed_ip_ids = [fip.id for fip in fixed_ips]
-    result = model_query(context, models.FixedIp, session=session,
-                         read_deleted="yes").\
+    result = session.query(models.FixedIp.id).\
+                     filter(models.FixedIp.deleted == False).\
+                     filter(models.FixedIp.allocated == False).\
+                     filter(models.FixedIp.updated_at < time).\
+                     join((models.Network,
+                           models.Network.id == models.FixedIp.network_id)).\
+                     join((models.Instance,
+                           models.Instance.id == models.FixedIp.instance_id)).\
+                     filter(host_filter).\
+                     all()
+    fixed_ip_ids = [fip[0] for fip in result]
+    if not fixed_ip_ids:
+        return 0
+    result = model_query(context, models.FixedIp, session=session).\
                      filter(models.FixedIp.id.in_(fixed_ip_ids)).\
                      update({'instance_id': None,
                              'leased': False,
diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py
index 9e5e3b0ada..8b73580b78 100644
--- a/nova/tests/test_db_api.py
+++ b/nova/tests/test_db_api.py
@@ -307,6 +307,56 @@ class DbApiTestCase(test.TestCase):
         data = db.network_get_associated_fixed_ips(ctxt, 1, 'nothing')
         self.assertEqual(len(data), 0)
 
+    def _timeout_test(self, ctxt, timeout, multi_host):
+        values = {'host': 'foo'}
+        instance = db.instance_create(ctxt, values)
+        values = {'multi_host': multi_host, 'host': 'bar'}
+        net = db.network_create_safe(ctxt, values)
+        old = time = timeout - datetime.timedelta(seconds=5)
+        new = time = timeout + datetime.timedelta(seconds=5)
+        # should deallocate
+        values = {'allocated': False,
+                  'instance_id': instance['id'],
+                  'network_id': net['id'],
+                  'updated_at': old}
+        db.fixed_ip_create(ctxt, values)
+        # still allocated
+        values = {'allocated': True,
+                  'instance_id': instance['id'],
+                  'network_id': net['id'],
+                  'updated_at': old}
+        db.fixed_ip_create(ctxt, values)
+        # wrong network
+        values = {'allocated': False,
+                  'instance_id': instance['id'],
+                  'network_id': None,
+                  'updated_at': old}
+        db.fixed_ip_create(ctxt, values)
+        # too new
+        values = {'allocated': False,
+                  'instance_id': instance['id'],
+                  'network_id': None,
+                  'updated_at': new}
+        db.fixed_ip_create(ctxt, values)
+
+    def test_fixed_ip_disassociate_all_by_timeout_single_host(self):
+        now = utils.utcnow()
+        ctxt = context.get_admin_context()
+        self._timeout_test(ctxt, now, False)
+        result = db.fixed_ip_disassociate_all_by_timeout(ctxt, 'foo', now)
+        self.assertEqual(result, 0)
+        result = db.fixed_ip_disassociate_all_by_timeout(ctxt, 'bar', now)
+        self.assertEqual(result, 1)
+
+    def test_fixed_ip_disassociate_all_by_timeout_multi_host(self):
+        now = utils.utcnow()
+        ctxt = context.get_admin_context()
+        self._timeout_test(ctxt, now, True)
+        result = db.fixed_ip_disassociate_all_by_timeout(ctxt, 'foo', now)
+        self.assertEqual(result, 1)
+        result = db.fixed_ip_disassociate_all_by_timeout(ctxt, 'bar', now)
+        self.assertEqual(result, 0)
+
 
 def _get_fake_aggr_values():
     return {'name': 'fake_aggregate',