Merge "Clean up read_deleted support in host aggregates code"

5 files changed, 46 insertions, 61 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 4452518be..d5d7723fa 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -1748,7 +1748,7 @@ class AggregateAPI(base.Base):
 
     def get_aggregate_list(self, context):
         """Get all the aggregates for this zone."""
-        aggregates = self.db.aggregate_get_all(context, read_deleted="no")
+        aggregates = self.db.aggregate_get_all(context)
         return [self._get_aggregate_info(context, a) for a in aggregates]
 
     def update_aggregate(self, context, aggregate_id, values):
@@ -1779,8 +1779,7 @@ class AggregateAPI(base.Base):
 
     def delete_aggregate(self, context, aggregate_id):
         """Deletes the aggregate."""
-        hosts = self.db.aggregate_host_get_all(context, aggregate_id,
-                                               read_deleted="no")
+        hosts = self.db.aggregate_host_get_all(context, aggregate_id)
         if len(hosts) > 0:
             raise exception.InvalidAggregateAction(action='delete',
                                                    aggregate_id=aggregate_id,
@@ -1845,9 +1844,7 @@ class AggregateAPI(base.Base):
     def _get_aggregate_info(self, context, aggregate):
         """Builds a dictionary with aggregate props, metadata and hosts."""
         metadata = self.db.aggregate_metadata_get(context, aggregate.id)
-        hosts = self.db.aggregate_host_get_all(context, aggregate.id,
-                                               read_deleted="no")
-
+        hosts = self.db.aggregate_host_get_all(context, aggregate.id)
         result = dict(aggregate.iteritems())
         result["metadata"] = metadata
         result["hosts"] = hosts
diff --git a/nova/db/api.py b/nova/db/api.py
index a7fbd34fd..f217a9e56 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -1707,14 +1707,14 @@ def aggregate_create(context, values, metadata=None):
     return IMPL.aggregate_create(context, values, metadata)
 
 
-def aggregate_get(context, aggregate_id, **kwargs):
+def aggregate_get(context, aggregate_id):
     """Get a specific aggregate by id."""
-    return IMPL.aggregate_get(context, aggregate_id, **kwargs)
+    return IMPL.aggregate_get(context, aggregate_id)
 
 
-def aggregate_get_by_host(context, host, **kwargs):
+def aggregate_get_by_host(context, host):
     """Get a specific aggregate by host"""
-    return IMPL.aggregate_get_by_host(context, host, **kwargs)
+    return IMPL.aggregate_get_by_host(context, host)
 
 
 def aggregate_update(context, aggregate_id, values):
@@ -1728,9 +1728,9 @@ def aggregate_delete(context, aggregate_id):
     return IMPL.aggregate_delete(context, aggregate_id)
 
 
-def aggregate_get_all(context, **kwargs):
+def aggregate_get_all(context):
     """Get all aggregates."""
-    return IMPL.aggregate_get_all(context, **kwargs)
+    return IMPL.aggregate_get_all(context)
 
 
 def aggregate_metadata_add(context, aggregate_id, metadata, set_delete=False):
@@ -1738,9 +1738,9 @@ def aggregate_metadata_add(context, aggregate_id, metadata, set_delete=False):
     IMPL.aggregate_metadata_add(context, aggregate_id, metadata, set_delete)
 
 
-def aggregate_metadata_get(context, aggregate_id, **kwargs):
+def aggregate_metadata_get(context, aggregate_id):
     """Get metadata for the specified aggregate."""
-    return IMPL.aggregate_metadata_get(context, aggregate_id, **kwargs)
+    return IMPL.aggregate_metadata_get(context, aggregate_id)
 
 
 def aggregate_metadata_delete(context, aggregate_id, key):
@@ -1753,9 +1753,9 @@ def aggregate_host_add(context, aggregate_id, host):
     IMPL.aggregate_host_add(context, aggregate_id, host)
 
 
-def aggregate_host_get_all(context, aggregate_id, **kwargs):
+def aggregate_host_get_all(context, aggregate_id):
     """Get hosts for the specified aggregate."""
-    return IMPL.aggregate_host_get_all(context, aggregate_id, **kwargs)
+    return IMPL.aggregate_host_get_all(context, aggregate_id)
 
 
 def aggregate_host_delete(context, aggregate_id, host):
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index a0224796b..ff1b5a3a4 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -4169,8 +4169,10 @@ def sm_volume_get_all(context):
 ################
 
 
-def _aggregate_get_query(context, model_class, id_field, id, **kwargs):
-    return model_query(context, model_class, **kwargs).filter(id_field == id)
+def _aggregate_get_query(context, model_class, id_field, id,
+                         session=None, read_deleted=None):
+    return model_query(context, model_class, session=session,
+                       read_deleted=read_deleted).filter(id_field == id)
 
 
 @require_admin_context
@@ -4200,12 +4202,11 @@ def aggregate_create(context, values, metadata=None):
 
 
 @require_admin_context
-def aggregate_get(context, aggregate_id, **kwargs):
+def aggregate_get(context, aggregate_id):
     aggregate = _aggregate_get_query(context,
                                      models.Aggregate,
                                      models.Aggregate.id,
-                                     aggregate_id,
-                                     **kwargs).first()
+                                     aggregate_id).first()
 
     if not aggregate:
         raise exception.AggregateNotFound(aggregate_id=aggregate_id)
@@ -4214,18 +4215,16 @@ def aggregate_get(context, aggregate_id, **kwargs):
 
 
 @require_admin_context
-def aggregate_get_by_host(context, host, **kwargs):
+def aggregate_get_by_host(context, host):
     aggregate_host = _aggregate_get_query(context,
                                           models.AggregateHost,
                                           models.AggregateHost.host,
-                                          host,
-                                          **kwargs).first()
+                                          host).first()
 
     if not aggregate_host:
         raise exception.AggregateHostNotFound(host=host)
 
-    return aggregate_get(context, aggregate_host.aggregate_id,
-                         **kwargs)
+    return aggregate_get(context, aggregate_host.aggregate_id)
 
 
 @require_admin_context
@@ -4233,9 +4232,9 @@ def aggregate_update(context, aggregate_id, values):
     session = get_session()
     aggregate = _aggregate_get_query(context,
                                      models.Aggregate,
-                                     models.Aggregate.id, aggregate_id,
-                                     session=session,
-                                     read_deleted='no').first()
+                                     models.Aggregate.id,
+                                     aggregate_id,
+                                     session=session).first()
     if aggregate:
         metadata = values.get('metadata')
         if metadata is not None:
@@ -4256,8 +4255,8 @@ def aggregate_update(context, aggregate_id, values):
 def aggregate_delete(context, aggregate_id):
     query = _aggregate_get_query(context,
                                  models.Aggregate,
-                                 models.Aggregate.id, aggregate_id,
-                                 read_deleted='no')
+                                 models.Aggregate.id,
+                                 aggregate_id)
     if query.first():
         query.update({'deleted': True,
                       'deleted_at': utils.utcnow(),
@@ -4268,20 +4267,16 @@ def aggregate_delete(context, aggregate_id):
 
 
 @require_admin_context
-def aggregate_get_all(context, **kwargs):
-    if 'read_deleted' not in kwargs:
-        kwargs['read_deleted'] = 'yes'
-    return model_query(context,
-                       models.Aggregate,
-                       **kwargs).all()
+def aggregate_get_all(context):
+    return model_query(context, models.Aggregate).all()
 
 
 @require_admin_context
 @require_aggregate_exists
-def aggregate_metadata_get(context, aggregate_id, **kwargs):
+def aggregate_metadata_get(context, aggregate_id):
     rows = model_query(context,
-                       models.AggregateMetadata,
-                       **kwargs).filter_by(aggregate_id=aggregate_id).all()
+                       models.AggregateMetadata).\
+                       filter_by(aggregate_id=aggregate_id).all()
 
     return dict([(r['key'], r['value']) for r in rows])
 
@@ -4292,7 +4287,7 @@ def aggregate_metadata_delete(context, aggregate_id, key):
     query = _aggregate_get_query(context,
                                  models.AggregateMetadata,
                                  models.AggregateMetadata.aggregate_id,
-                                 aggregate_id, read_deleted='no').\
+                                 aggregate_id).\
                                  filter_by(key=key)
     if query.first():
         query.update({'deleted': True,
@@ -4305,8 +4300,7 @@ def aggregate_metadata_delete(context, aggregate_id, key):
 
 @require_admin_context
 @require_aggregate_exists
-def aggregate_metadata_get_item(context, aggregate_id, key,
-                                session=None):
+def aggregate_metadata_get_item(context, aggregate_id, key, session=None):
     result = _aggregate_get_query(context,
                                   models.AggregateMetadata,
                                   models.AggregateMetadata.aggregate_id,
@@ -4356,12 +4350,10 @@ def aggregate_metadata_add(context, aggregate_id, metadata, set_delete=False):
 
 @require_admin_context
 @require_aggregate_exists
-def aggregate_host_get_all(context, aggregate_id, **kwargs):
-    if 'read_deleted' not in kwargs:
-        kwargs['read_deleted'] = 'yes'
+def aggregate_host_get_all(context, aggregate_id):
     rows = model_query(context,
-                       models.AggregateHost,
-                       **kwargs).filter_by(aggregate_id=aggregate_id).all()
+                       models.AggregateHost).\
+                       filter_by(aggregate_id=aggregate_id).all()
 
     return [r.host for r in rows]
 
@@ -4372,8 +4364,7 @@ def aggregate_host_delete(context, aggregate_id, host):
     query = _aggregate_get_query(context,
                                  models.AggregateHost,
                                  models.AggregateHost.aggregate_id,
-                                 aggregate_id,
-                                 read_deleted='no').filter_by(host=host)
+                                 aggregate_id).filter_by(host=host)
     if query.first():
         query.update({'deleted': True,
                       'deleted_at': utils.utcnow(),
diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py
index 2e7e7de9b..27fadc321 100644
--- a/nova/tests/test_compute.py
+++ b/nova/tests/test_compute.py
@@ -3286,8 +3286,8 @@ class ComputeAPIAggrTestCase(test.TestCase):
         aggr = self.api.create_aggregate(self.context, 'fake_aggregate',
                                          'fake_zone')
         self.api.delete_aggregate(self.context, aggr['id'])
-        expected = db.aggregate_get(self.context, aggr['id'],
-                                    read_deleted='yes')
+        expected = db.aggregate_get(self.context.elevated(read_deleted='yes'),
+                                    aggr['id'])
         self.assertNotEqual(aggr['operational_state'],
                             expected['operational_state'])
 
diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py
index b7b9531c3..b5607cee4 100644
--- a/nova/tests/test_db_api.py
+++ b/nova/tests/test_db_api.py
@@ -503,11 +503,10 @@ class AggregateDBApiTestCase(test.TestCase):
         ctxt = context.get_admin_context()
         result = _create_aggregate(context=ctxt, metadata=None)
         db.aggregate_delete(ctxt, result['id'])
-        expected = db.aggregate_get_all(ctxt, read_deleted='no')
+        expected = db.aggregate_get_all(ctxt)
         self.assertEqual(0, len(expected))
-
-        ctxt = context.get_admin_context(read_deleted='yes')
-        aggregate = db.aggregate_get(ctxt, result['id'])
+        aggregate = db.aggregate_get(ctxt.elevated(read_deleted='yes'),
+                                     result['id'])
         self.assertEqual(aggregate["operational_state"], "dismissed")
 
     def test_aggregate_update(self):
@@ -575,7 +574,7 @@ class AggregateDBApiTestCase(test.TestCase):
                                                 values=values, metadata=None))
         for c in xrange(1, remove_counter):
             db.aggregate_delete(ctxt, aggregates[c - 1].id)
-        results = db.aggregate_get_all(ctxt, read_deleted='no')
+        results = db.aggregate_get_all(ctxt)
         self.assertEqual(len(results), add_counter - remove_counter)
 
     def test_aggregate_metadata_add(self):
@@ -633,8 +632,7 @@ class AggregateDBApiTestCase(test.TestCase):
         host = _get_fake_aggr_hosts()[0]
         db.aggregate_host_delete(ctxt, result.id, host)
         db.aggregate_host_add(ctxt, result.id, host)
-        expected = db.aggregate_host_get_all(ctxt, result.id,
-                                             read_deleted='no')
+        expected = db.aggregate_host_get_all(ctxt, result.id)
         self.assertEqual(len(expected), 1)
 
     def test_aggregate_host_add_duplicate_raise_conflict(self):
@@ -671,8 +669,7 @@ class AggregateDBApiTestCase(test.TestCase):
         result = _create_aggregate_with_hosts(context=ctxt, metadata=None)
         db.aggregate_host_delete(ctxt, result.id,
                                  _get_fake_aggr_hosts()[0])
-        expected = db.aggregate_host_get_all(ctxt, result.id,
-                                             read_deleted='no')
+        expected = db.aggregate_host_get_all(ctxt, result.id)
         self.assertEqual(0, len(expected))
 
     def test_aggregate_host_delete_raise_not_found(self):