From 1155b734164eb5856d68c926f7bf64a37ae4a3a4 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Tue, 30 Aug 2011 11:13:25 -0400 Subject: [PATCH 1/3] supporting changes-since --- nova/api/openstack/servers.py | 24 +++++++++++++----------- nova/db/sqlalchemy/api.py | 12 ++++++++---- nova/tests/api/openstack/test_servers.py | 24 ++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 15 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 27c67e79e858..e0e40679a7f8 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -107,6 +107,14 @@ class Controller(object): LOG.error(reason) raise exception.InvalidInput(reason=reason) + if 'changes-since' in search_opts: + try: + parsed = utils.parse_isotime(search_opts['changes-since']) + except ValueError: + msg = _('Invalid changes-since value') + raise exc.HTTPBadRequest(explanation=msg) + search_opts['changes-since'] = parsed + # By default, compute's get_all() will return deleted instances. # If an admin hasn't specified a 'deleted' search option, we need # to filter out deleted instances by setting the filter ourselves. @@ -114,23 +122,17 @@ class Controller(object): # should return recently deleted images according to the API spec. if 'deleted' not in search_opts: - # Admin hasn't specified deleted filter if 'changes-since' not in search_opts: - # No 'changes-since', so we need to find non-deleted servers + # No 'changes-since', so we only want non-deleted servers search_opts['deleted'] = False - else: - # This is the default, but just in case.. - search_opts['deleted'] = True - instance_list = self.compute_api.get_all( - context, search_opts=search_opts) - - # FIXME(comstud): 'changes-since' is not fully implemented. Where - # should this be filtered? + instance_list = self.compute_api.get_all(context, + search_opts=search_opts) limited_list = self._limit_items(instance_list, req) servers = [self._build_view(req, inst, is_detail)['server'] - for inst in limited_list] + for inst in limited_list] + return dict(servers=servers) @scheduler_api.redirect_handler diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 65b09a65dd72..1685e9928506 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -35,6 +35,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 desc from sqlalchemy.sql.expression import literal_column FLAGS = flags.FLAGS @@ -1250,12 +1251,17 @@ def instance_get_all_by_filters(context, filters): options(joinedload_all('fixed_ips.network')).\ options(joinedload('metadata')).\ options(joinedload('instance_type')).\ - filter_by(deleted=can_read_deleted(context)) + order_by(desc(models.Instance.updated_at)) # 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. filters = filters.copy() + if 'changes-since' in filters: + changes_since = filters['changes-since'] + query_prefix = query_prefix.\ + filter(models.Instance.updated_at > changes_since) + if not context.is_admin: # If we're not admin context, add appropriate filter.. if context.project_id: @@ -1277,9 +1283,7 @@ def instance_get_all_by_filters(context, filters): query_prefix = _exact_match_filter(query_prefix, filter_name, filters.pop(filter_name)) - instances = query_prefix.\ - filter_by(deleted=can_read_deleted(context)).\ - all() + instances = query_prefix.all() if not instances: return [] diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 3559e6de5957..9036d6552565 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1257,6 +1257,30 @@ class ServersTest(test.TestCase): self.assertEqual(len(servers), 1) self.assertEqual(servers[0]['id'], 100) + def test_get_servers_allows_changes_since_v1_1(self): + def fake_get_all(compute_self, context, search_opts=None): + self.assertNotEqual(search_opts, None) + self.assertTrue('changes-since' in search_opts) + changes_since = datetime.datetime(2011, 1, 24, 17, 8, 1) + self.assertEqual(search_opts['changes-since'], changes_since) + return [stub_instance(100)] + + self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + + params = 'changes-since=2011-01-24T17:08:01Z' + req = webob.Request.blank('/v1.1/fake/servers?%s' % params) + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 200) + servers = json.loads(res.body)['servers'] + self.assertEqual(len(servers), 1) + self.assertEqual(servers[0]['id'], 100) + + def test_get_servers_allows_changes_since_bad_value_v1_1(self): + params = 'changes-since=asdf' + req = webob.Request.blank('/v1.1/fake/servers?%s' % params) + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 400) + def test_get_servers_unknown_or_admin_options1(self): """Test getting servers by admin-only or unknown options. This tests when admin_api is off. Make sure the admin and From a127747db0ab3405a768e8f680a2eb94ae8ce314 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Tue, 30 Aug 2011 11:44:19 -0400 Subject: [PATCH 2/3] adding an assert --- nova/tests/api/openstack/test_servers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 9036d6552565..5f1ca466a9f9 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1263,6 +1263,7 @@ class ServersTest(test.TestCase): self.assertTrue('changes-since' in search_opts) changes_since = datetime.datetime(2011, 1, 24, 17, 8, 1) self.assertEqual(search_opts['changes-since'], changes_since) + self.assertTrue('deleted' not in search_opts) return [stub_instance(100)] self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) From 1c6d74a08dbb5b472e85e3d3a1fe2b3b8b9b89e3 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Tue, 30 Aug 2011 12:51:02 -0400 Subject: [PATCH 3/3] changing default sort to created_at --- nova/db/sqlalchemy/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 1685e9928506..1e55d08e7a20 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1251,7 +1251,7 @@ def instance_get_all_by_filters(context, filters): options(joinedload_all('fixed_ips.network')).\ options(joinedload('metadata')).\ options(joinedload('instance_type')).\ - order_by(desc(models.Instance.updated_at)) + order_by(desc(models.Instance.created_at)) # 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.