improve session handling around instance_ methods

clean up session usage in some of the instance_* methods:
  instance_destroy
  instance_get_all_hung_in_rebooting
  instance_(add|remove)_security_group
  instance_info_cache_(get|update|delete)

also remove instance_info_cache_create since this is not used

blueprint db-session-cleanup

Change-Id: I33913dae3bc28e05d856d951454440a12616706e

2 files changed, 41 insertions, 77 deletions

diff --git a/nova/db/api.py b/nova/db/api.py
index 51d6dc6ab..e58bac69c 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -702,15 +702,6 @@ def instance_remove_security_group(context, instance_id, security_group_id):
 ###################
 
 
-def instance_info_cache_create(context, values):
-    """Create a new instance cache record in the table.
-
-    :param context: = request context object
-    :param values: = dict containing column values
-    """
-    return IMPL.instance_info_cache_create(context, values)
-
-
 def instance_info_cache_get(context, instance_uuid):
     """Gets an instance info cache from the table.
 
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index eb1247e7d..0b0b35c8c 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -37,6 +37,7 @@ from sqlalchemy.sql import func
 
 from nova import block_device
 from nova.common.sqlalchemyutils import paginate_query
+from nova.compute import task_states
 from nova.compute import vm_states
 from nova import config
 from nova import db
@@ -1428,7 +1429,7 @@ def instance_destroy(context, instance_uuid, constraint=None):
             raise exception.InvalidUUID(instance_uuid)
 
         query = session.query(models.Instance).\
-                        filter_by(uuid=instance_ref['uuid'])
+                        filter_by(uuid=instance_uuid)
         if constraint is not None:
             query = constraint.apply(models.Instance, query)
         count = query.update({'deleted': True,
@@ -1437,13 +1438,16 @@ def instance_destroy(context, instance_uuid, constraint=None):
         if count == 0:
             raise exception.ConstraintNotMet()
         session.query(models.SecurityGroupInstanceAssociation).\
-                filter_by(instance_uuid=instance_ref['uuid']).\
+                filter_by(instance_uuid=instance_uuid).\
                 update({'deleted': True,
                         'deleted_at': timeutils.utcnow(),
                         'updated_at': literal_column('updated_at')})
 
-        instance_info_cache_delete(context, instance_ref['uuid'],
-                                   session=session)
+        session.query(models.InstanceInfoCache).\
+                 filter_by(instance_uuid=instance_uuid).\
+                 update({'deleted': True,
+                         'deleted_at': timeutils.utcnow(),
+                         'updated_at': literal_column('updated_at')})
     return instance_ref
 
 
@@ -1722,18 +1726,13 @@ def instance_floating_address_get_all(context, instance_uuid):
 
 
 @require_admin_context
-def instance_get_all_hung_in_rebooting(context, reboot_window, session=None):
+def instance_get_all_hung_in_rebooting(context, reboot_window):
     reboot_window = (timeutils.utcnow() -
                      datetime.timedelta(seconds=reboot_window))
 
-    if not session:
-        session = get_session()
-
-    results = session.query(models.Instance).\
+    return model_query(context, models.Instance).\
             filter(models.Instance.updated_at <= reboot_window).\
-            filter_by(task_state="rebooting").all()
-
-    return results
+            filter_by(task_state=task_states.REBOOTING).all()
 
 
 @require_context
@@ -1849,25 +1848,17 @@ def _instance_update(context, instance_uuid, values, copy_old_instance=False):
 
 def instance_add_security_group(context, instance_uuid, security_group_id):
     """Associate the given security group with the given instance"""
-    session = get_session()
-    with session.begin():
-        instance_ref = instance_get_by_uuid(context, instance_uuid,
-                                            session=session)
-        security_group_ref = security_group_get(context,
-                                                security_group_id,
-                                                session=session)
-        instance_ref.security_groups += [security_group_ref]
-        instance_ref.save(session=session)
+    sec_group_ref = models.SecurityGroupInstanceAssociation()
+    sec_group_ref.update({'instance_uuid': instance_uuid,
+                          'security_group_id': security_group_id})
+    sec_group_ref.save()
 
 
 @require_context
 def instance_remove_security_group(context, instance_uuid, security_group_id):
     """Disassociate the given security group from the given instance"""
-    session = get_session()
-    instance_ref = instance_get_by_uuid(context, instance_uuid,
-                                        session=session)
-    session.query(models.SecurityGroupInstanceAssociation).\
-                filter_by(instance_uuid=instance_ref['uuid']).\
+    model_query(context, models.SecurityGroupInstanceAssociation).\
+                filter_by(instance_uuid=instance_uuid).\
                 filter_by(security_group_id=security_group_id).\
                 update({'deleted': True,
                         'deleted_at': timeutils.utcnow(),
@@ -1878,74 +1869,56 @@ def instance_remove_security_group(context, instance_uuid, security_group_id):
 
 
 @require_context
-def instance_info_cache_create(context, values):
-    """Create a new instance cache record in the table.
-
-    :param context: = request context object
-    :param values: = dict containing column values
-    """
-    info_cache = models.InstanceInfoCache()
-    info_cache.update(values)
-
-    session = get_session()
-    with session.begin():
-        info_cache.save(session=session)
-    return info_cache
-
-
-@require_context
-def instance_info_cache_get(context, instance_uuid, session=None):
+def instance_info_cache_get(context, instance_uuid):
     """Gets an instance info cache from the table.
 
     :param instance_uuid: = uuid of the info cache's instance
     :param session: = optional session object
     """
-    session = session or get_session()
-
-    info_cache = session.query(models.InstanceInfoCache).\
+    return model_query(context, models.InstanceInfoCache).\
                          filter_by(instance_uuid=instance_uuid).\
                          first()
-    return info_cache
 
 
 @require_context
-def instance_info_cache_update(context, instance_uuid, values,
-                               session=None):
+def instance_info_cache_update(context, instance_uuid, values):
     """Update an instance info cache record in the table.
 
     :param instance_uuid: = uuid of info cache's instance
     :param values: = dict containing column values to update
     :param session: = optional session object
     """
-    session = session or get_session()
-    info_cache = instance_info_cache_get(context, instance_uuid,
-                                         session=session)
-    if info_cache:
-        # NOTE(tr3buchet): let's leave it alone if it's already deleted
-        if info_cache['deleted']:
-            return info_cache
-
-        info_cache.update(values)
-        info_cache.save(session=session)
-    else:
-        # NOTE(tr3buchet): just in case someone blows away an instance's
-        #                  cache entry
-        values['instance_uuid'] = instance_uuid
-        info_cache = instance_info_cache_create(context, values)
+    session = get_session()
+    with session.begin():
+        info_cache = model_query(context, models.InstanceInfoCache,
+                                 session=session).\
+                         filter_by(instance_uuid=instance_uuid).\
+                         first()
+
+        if info_cache and not info_cache['deleted']:
+            # NOTE(tr3buchet): let's leave it alone if it's already deleted
+            info_cache.update(values)
+        else:
+            # NOTE(tr3buchet): just in case someone blows away an instance's
+            #                  cache entry
+            info_cache = models.InstanceInfoCache()
+            info_cache.update({'instance_uuid': instance_uuid})
 
     return info_cache
 
 
 @require_context
-def instance_info_cache_delete(context, instance_uuid, session=None):
+def instance_info_cache_delete(context, instance_uuid):
     """Deletes an existing instance_info_cache record
 
     :param instance_uuid: = uuid of the instance tied to the cache record
     :param session: = optional session object
     """
-    values = {'deleted': True,
-              'deleted_at': timeutils.utcnow()}
-    instance_info_cache_update(context, instance_uuid, values, session)
+    model_query(context, models.InstanceInfoCache).\
+                         filter_by(instance_uuid=instance_uuid).\
+                         update({'deleted': True,
+                                 'deleted_at': timeutils.utcnow(),
+                                 'updated_at': literal_column('updated_at')})
 
 
 ###################