openstack/nova
Code Issues Proposed changes

Merge "Sort results from describe_instances in EC2 API."

Browse Source
This commit is contained in:
Jenkins 2012-03-13 16:59:53 +00:00 committed by Gerrit Code Review
commit 094985ea65
6 changed files with 107 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
nova/api/ec2/cloud.py
View File

@ -1160,7 +1160,8 @@ class CloudController(object):
# always filter out deleted instances
search_opts['deleted'] = False
instances = self.compute_api.get_all(context,
search_opts=search_opts)
search_opts=search_opts,
sort_dir='asc')
except exception.NotFound:
instances = []
for instance in instances:

15
nova/compute/api.py
View File

@ -1028,7 +1028,8 @@ class API(base.Base):
inst['name'] = instance['name']
return inst
def get_all(self, context, search_opts=None):
def get_all(self, context, search_opts=None, sort_key='created_at',
sort_dir='desc'):
"""Get all instances filtered by one of the given parameters.
If there is no filter and the context is an admin, it will retrieve
@ -1036,6 +1037,10 @@ class API(base.Base):
Deleted instances will be returned by default, unless there is a
search option that says otherwise.
The results will be returned sorted in the order specified by the
'sort_dir' parameter using the key specified in the 'sort_key'
parameter.
"""
#TODO(bcwaldon): determine the best argument for target here
@ -1101,7 +1106,8 @@ class API(base.Base):
except ValueError:
return []
inst_models = self._get_instances_by_filters(context, filters)
inst_models = self._get_instances_by_filters(context, filters,
sort_key, sort_dir)
# Convert the models to dictionaries
instances = []
@ -1113,7 +1119,7 @@ class API(base.Base):
return instances
def _get_instances_by_filters(self, context, filters):
def _get_instances_by_filters(self, context, filters, sort_key, sort_dir):
if 'ip6' in filters or 'ip' in filters:
res = self.network_api.get_instance_uuids_by_ip_filter(context,
filters)
@ -1122,7 +1128,8 @@ class API(base.Base):
uuids = set([r['instance_uuid'] for r in res])
filters['uuid'] = uuids
return self.db.instance_get_all_by_filters(context, filters)
return self.db.instance_get_all_by_filters(context, filters, sort_key,
sort_dir)
@wrap_check_policy
@check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.SHUTOFF])

6
nova/db/api.py
View File

@ -562,9 +562,11 @@ def instance_get_all(context):
return IMPL.instance_get_all(context)
def instance_get_all_by_filters(context, filters):
def instance_get_all_by_filters(context, filters, sort_key='created_at',
sort_dir='desc'):
"""Get all instances that match all filters."""
return IMPL.instance_get_all_by_filters(context, filters)
return IMPL.instance_get_all_by_filters(context, filters, sort_key,
sort_dir)
def instance_get_active_by_window(context, begin, end=None, project_id=None):

7
nova/db/sqlalchemy/api.py
View File

@ -40,6 +40,7 @@ from sqlalchemy.exc import IntegrityError
from sqlalchemy.orm import joinedload
from sqlalchemy.orm import joinedload_all
from sqlalchemy.sql import func
from sqlalchemy.sql.expression import asc
from sqlalchemy.sql.expression import desc
from sqlalchemy.sql.expression import literal_column
@ -1379,7 +1380,7 @@ def instance_get_all(context):
@require_context
def instance_get_all_by_filters(context, filters):
def instance_get_all_by_filters(context, filters, sort_key, sort_dir):
"""Return instances that match all filters. Deleted instances
will be returned by default, unless there's a filter that says
otherwise"""
@ -1406,13 +1407,15 @@ def instance_get_all_by_filters(context, filters):
return True
return False
sort_fn = {'desc': desc, 'asc': asc}
session = get_session()
query_prefix = session.query(models.Instance).\
options(joinedload('info_cache')).\
options(joinedload('security_groups')).\
options(joinedload('metadata')).\
options(joinedload('instance_type')).\
order_by(desc(models.Instance.created_at))
order_by(sort_fn[sort_dir](getattr(models.Instance, sort_key)))
# Make a copy of the filters dictionary to use going forward, as we'll
# be modifying it and we shouldn't affect the caller's use of it.

57
nova/tests/api/ec2/test_cloud.py
View File

@ -19,6 +19,7 @@
import base64
import copy
import datetime
import functools
import os
import string
@ -712,6 +713,62 @@ class CloudTestCase(test.TestCase):
db.service_destroy(self.context, comp1['id'])
db.service_destroy(self.context, comp2['id'])
def test_describe_instances_sorting(self):
"""Makes sure describe_instances works and is sorted as expected."""
self.flags(use_ipv6=True)
self._stub_instance_get_with_fixed_ips('get_all')
self._stub_instance_get_with_fixed_ips('get')
image_uuid = 'cedef40a-ed67-4d10-800e-17455edce175'
inst_base = {
'reservation_id': 'a',
'image_ref': image_uuid,
'instance_type_id': 1,
'vm_state': 'active'
}
inst1_kwargs = {}
inst1_kwargs.update(inst_base)
inst1_kwargs['host'] = 'host1'
inst1_kwargs['hostname'] = 'server-1111'
inst1_kwargs['created_at'] = datetime.datetime(2012, 5, 1, 1, 1, 1)
inst1 = db.instance_create(self.context, inst1_kwargs)
inst2_kwargs = {}
inst2_kwargs.update(inst_base)
inst2_kwargs['host'] = 'host2'
inst2_kwargs['hostname'] = 'server-2222'
inst2_kwargs['created_at'] = datetime.datetime(2012, 2, 1, 1, 1, 1)
inst2 = db.instance_create(self.context, inst2_kwargs)
inst3_kwargs = {}
inst3_kwargs.update(inst_base)
inst3_kwargs['host'] = 'host3'
inst3_kwargs['hostname'] = 'server-3333'
inst3_kwargs['created_at'] = datetime.datetime(2012, 2, 5, 1, 1, 1)
inst3 = db.instance_create(self.context, inst3_kwargs)
comp1 = db.service_create(self.context, {'host': 'host1',
'availability_zone': 'zone1',
'topic': "compute"})
comp2 = db.service_create(self.context, {'host': 'host2',
'availability_zone': 'zone2',
'topic': "compute"})
result = self.cloud.describe_instances(self.context)
result = result['reservationSet'][0]['instancesSet']
self.assertEqual(result[0]['launchTime'], inst2_kwargs['created_at'])
self.assertEqual(result[1]['launchTime'], inst3_kwargs['created_at'])
self.assertEqual(result[2]['launchTime'], inst1_kwargs['created_at'])
db.instance_destroy(self.context, inst1['id'])
db.instance_destroy(self.context, inst2['id'])
db.instance_destroy(self.context, inst3['id'])
db.service_destroy(self.context, comp1['id'])
db.service_destroy(self.context, comp2['id'])
def test_describe_instance_state(self):
"""Makes sure describe_instances for instanceState works."""

42
nova/tests/api/openstack/compute/test_servers.py
View File

@ -567,7 +567,8 @@ class ServersControllerTest(test.TestCase):
def test_get_servers_with_bad_option(self):
server_uuid = str(utils.gen_uuid())
def fake_get_all(compute_self, context, search_opts=None):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc'):
return [fakes.stub_instance(100, uuid=server_uuid)]
self.stubs.Set(nova.compute.API, 'get_all', fake_get_all)
@ -581,7 +582,8 @@ class ServersControllerTest(test.TestCase):
def test_get_servers_allows_image(self):
server_uuid = str(utils.gen_uuid())
def fake_get_all(compute_self, context, search_opts=None):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc'):
self.assertNotEqual(search_opts, None)
self.assertTrue('image' in search_opts)
self.assertEqual(search_opts['image'], '12345')
@ -596,7 +598,8 @@ class ServersControllerTest(test.TestCase):
self.assertEqual(servers[0]['id'], server_uuid)
def test_tenant_id_filter_converts_to_project_id_for_admin(self):
def fake_get_all(context, filters=None, instances=None):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc'):
self.assertNotEqual(filters, None)
self.assertEqual(filters['project_id'], 'fake')
self.assertFalse(filters.get('tenant_id'))
@ -612,7 +615,8 @@ class ServersControllerTest(test.TestCase):
self.assertTrue('servers' in res)
def test_admin_restricted_tenant(self):
def fake_get_all(context, filters=None, instances=None):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc'):
self.assertNotEqual(filters, None)
self.assertEqual(filters['project_id'], 'fake')
return [fakes.stub_instance(100)]
@ -627,7 +631,8 @@ class ServersControllerTest(test.TestCase):
self.assertTrue('servers' in res)
def test_admin_all_tenants(self):
def fake_get_all(context, filters=None, instances=None):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc'):
self.assertNotEqual(filters, None)
self.assertTrue('project_id' not in filters)
return [fakes.stub_instance(100)]
@ -642,7 +647,8 @@ class ServersControllerTest(test.TestCase):
self.assertTrue('servers' in res)
def test_all_tenants(self):
def fake_get_all(context, filters=None, instances=None):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc'):
self.assertNotEqual(filters, None)
self.assertEqual(filters['project_id'], 'fake')
return [fakes.stub_instance(100)]
@ -658,7 +664,8 @@ class ServersControllerTest(test.TestCase):
def test_get_servers_allows_flavor(self):
server_uuid = str(utils.gen_uuid())
def fake_get_all(compute_self, context, search_opts=None):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc'):
self.assertNotEqual(search_opts, None)
self.assertTrue('flavor' in search_opts)
# flavor is an integer ID
@ -676,7 +683,8 @@ class ServersControllerTest(test.TestCase):
def test_get_servers_allows_status(self):
server_uuid = str(utils.gen_uuid())
def fake_get_all(compute_self, context, search_opts=None):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc'):
self.assertNotEqual(search_opts, None)
self.assertTrue('vm_state' in search_opts)
self.assertEqual(search_opts['vm_state'], vm_states.ACTIVE)
@ -699,7 +707,8 @@ class ServersControllerTest(test.TestCase):
def test_get_servers_allows_name(self):
server_uuid = str(utils.gen_uuid())
def fake_get_all(compute_self, context, search_opts=None):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc'):
self.assertNotEqual(search_opts, None)
self.assertTrue('name' in search_opts)
self.assertEqual(search_opts['name'], 'whee.*')
@ -716,7 +725,8 @@ class ServersControllerTest(test.TestCase):
def test_get_servers_allows_changes_since(self):
server_uuid = str(utils.gen_uuid())
def fake_get_all(compute_self, context, search_opts=None):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc'):
self.assertNotEqual(search_opts, None)
self.assertTrue('changes-since' in search_opts)
changes_since = datetime.datetime(2011, 1, 24, 17, 8, 1,
@ -746,7 +756,8 @@ class ServersControllerTest(test.TestCase):
"""
server_uuid = str(utils.gen_uuid())
def fake_get_all(compute_self, context, search_opts=None):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc'):
self.assertNotEqual(search_opts, None)
# Allowed by user
self.assertTrue('name' in search_opts)
@ -773,7 +784,8 @@ class ServersControllerTest(test.TestCase):
"""
server_uuid = str(utils.gen_uuid())
def fake_get_all(compute_self, context, search_opts=None):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc'):
self.assertNotEqual(search_opts, None)
# Allowed by user
self.assertTrue('name' in search_opts)
@ -800,7 +812,8 @@ class ServersControllerTest(test.TestCase):
"""
server_uuid = str(utils.gen_uuid())
def fake_get_all(compute_self, context, search_opts=None):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc'):
self.assertNotEqual(search_opts, None)
self.assertTrue('ip' in search_opts)
self.assertEqual(search_opts['ip'], '10\..*')
@ -821,7 +834,8 @@ class ServersControllerTest(test.TestCase):
"""
server_uuid = str(utils.gen_uuid())
def fake_get_all(compute_self, context, search_opts=None):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc'):
self.assertNotEqual(search_opts, None)
self.assertTrue('ip6' in search_opts)
self.assertEqual(search_opts['ip6'], 'ffff.*')