OpenStack Compute (nova)

Merge lp:~rackspace-titan/nova/servers-next into lp:~hudson-openstack/nova/trunk

  1. servers-next
  2. Merge into trunk
Proposed by William Wolf
Status: Merged
Approved by: Brian Lamar
Approved revision: 1589
Merged at revision: 1615
Proposed branch: lp:~rackspace-titan/nova/servers-next
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 362 lines (+216/-9)
5 files modified
nova/api/openstack/common.py (+10/-0)
nova/api/openstack/schemas/v1.1/servers_index.rng (+3/-0)
nova/api/openstack/servers.py (+35/-7)
nova/api/openstack/views/servers.py (+36/-1)
nova/tests/api/openstack/test_servers.py (+132/-1)
To merge this branch: bzr merge lp:~rackspace-titan/nova/servers-next
Reviewer Review Type Date Requested Status
Brian Lamar (community) Approve
Brian Waldon (community) Approve
Review via email: mp+75558@code.launchpad.net

Description of the change

Add next links for server lists in OSAPI 1.1. This adds servers_links to the json responses, and an extra atom:link element to the servers node in the xml response.

To post a comment you must log in.
Revision history for this message
Brian Waldon (bcwaldon) wrote :

When making a request with a changes-since query param such as this:

curl -i -H "X-Auth-Token: admin:admin" 'http://localhost:8774/v1.1/admin/servers?limit=2&changes-since=2011-09-01T00:00:00Z'

The values of changes-since and limit are respected, but the servers_links value returned is unusable:

http://localhost:8774/v1.1?marker=6&limit=2&changes-since=2011-09-01T00%3A00%3A00Z

I would expect the changes-since value not to be url-encoded at all.

review: Needs Fixing
lp:~rackspace-titan/nova/servers-next updated
1582. By William Wolf

merge with trunk

1583. By William Wolf

make our own function instead of using urllib.urlencode since we apparently don't suppor urlencoded strings yet

1584. By William Wolf

merge with trunk

Revision history for this message
William Wolf (throughnothing) wrote :

I've replaced urllib with a common function that doesn't urlencode the strings from the dict. I think ultimately we should support urlencoded URL's though, and give those in our own links just to ensure everything is correctly set/interpreted.

Revision history for this message
Brian Waldon (bcwaldon) wrote :

I wouldn't call it elegant, but it definitely works. However, now I'm getting the following url back. Notice the missing 'admin/servers' after 'v1.1':

http://localhost:8774/v1.1?marker=8&limit=1&changes-since=2011-09-01T00:00:00Z

Revision history for this message
William Wolf (throughnothing) wrote :

Wow, good catch waldon. Seems I wasn't even checking that in the tests either. Updated the tests and the code, that should be fixed now.

lp:~rackspace-titan/nova/servers-next updated
1585. By William Wolf

remove urllib import

1586. By William Wolf

oops, add project_id and 'servers' to next links

1587. By William Wolf

merged with trunk

1588. By William Wolf

merge from trunk

1589. By William Wolf

pep8 fixes

Revision history for this message
Brian Waldon (bcwaldon) wrote :

Great stuff.

review: Approve
Revision history for this message
Brian Lamar (blamar) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'nova/api/openstack/common.py'
--- nova/api/openstack/common.py 2011-09-14 15:23:11 +0000
+++ nova/api/openstack/common.py 2011-09-20 18:47:17 +0000
@@ -259,6 +259,16 @@
259 headers={'Retry-After': 0})259 headers={'Retry-After': 0})
260260
261261
262def dict_to_query_str(params):
263 # TODO: we should just use urllib.urlencode instead of this
264 # But currently we don't work with urlencoded url's
265 param_str = ""
266 for key, val in params.iteritems():
267 param_str = param_str + '='.join([str(key), str(val)]) + '&'
268
269 return param_str.rstrip('&')
270
271
262class MetadataXMLDeserializer(wsgi.XMLDeserializer):272class MetadataXMLDeserializer(wsgi.XMLDeserializer):
263273
264 def extract_metadata(self, metadata_node):274 def extract_metadata(self, metadata_node):
265275
=== modified file 'nova/api/openstack/schemas/v1.1/servers_index.rng'
--- nova/api/openstack/schemas/v1.1/servers_index.rng 2011-08-19 19:55:56 +0000
+++ nova/api/openstack/schemas/v1.1/servers_index.rng 2011-09-20 18:47:17 +0000
@@ -9,4 +9,7 @@
9 </zeroOrMore>9 </zeroOrMore>
10 </element>10 </element>
11 </zeroOrMore>11 </zeroOrMore>
12 <zeroOrMore>
13 <externalRef href="../atom-link.rng"/>
14 </zeroOrMore>
12</element>15</element>
1316
=== modified file 'nova/api/openstack/servers.py'
--- nova/api/openstack/servers.py 2011-09-18 21:01:44 +0000
+++ nova/api/openstack/servers.py 2011-09-20 18:47:17 +0000
@@ -75,6 +75,9 @@
75 def _build_view(self, req, instance, is_detail=False):75 def _build_view(self, req, instance, is_detail=False):
76 raise NotImplementedError()76 raise NotImplementedError()
7777
78 def _build_list(self, req, instances, is_detail=False):
79 raise NotImplementedError()
80
78 def _limit_items(self, items, req):81 def _limit_items(self, items, req):
79 raise NotImplementedError()82 raise NotImplementedError()
8083
@@ -130,10 +133,7 @@
130 search_opts=search_opts)133 search_opts=search_opts)
131134
132 limited_list = self._limit_items(instance_list, req)135 limited_list = self._limit_items(instance_list, req)
133 servers = [self._build_view(req, inst, is_detail)['server']136 return self._build_list(req, limited_list, is_detail=is_detail)
134 for inst in limited_list]
135
136 return dict(servers=servers)
137137
138 @scheduler_api.redirect_handler138 @scheduler_api.redirect_handler
139 def show(self, req, id):139 def show(self, req, id):
@@ -595,6 +595,11 @@
595 builder = nova.api.openstack.views.servers.ViewBuilderV10(addresses)595 builder = nova.api.openstack.views.servers.ViewBuilderV10(addresses)
596 return builder.build(instance, is_detail=is_detail)596 return builder.build(instance, is_detail=is_detail)
597597
598 def _build_list(self, req, instances, is_detail=False):
599 addresses = nova.api.openstack.views.addresses.ViewBuilderV10()
600 builder = nova.api.openstack.views.servers.ViewBuilderV10(addresses)
601 return builder.build_list(instances, is_detail=is_detail)
602
598 def _limit_items(self, items, req):603 def _limit_items(self, items, req):
599 return common.limited(items, req)604 return common.limited(items, req)
600605
@@ -692,6 +697,25 @@
692697
693 return builder.build(instance, is_detail=is_detail)698 return builder.build(instance, is_detail=is_detail)
694699
700 def _build_list(self, req, instances, is_detail=False):
701 params = req.GET.copy()
702 pagination_params = common.get_pagination_params(req)
703 # Update params with int() values from pagination params
704 for key, val in pagination_params.iteritems():
705 params[key] = val
706
707 project_id = getattr(req.environ['nova.context'], 'project_id', '')
708 base_url = req.application_url
709 flavor_builder = nova.api.openstack.views.flavors.ViewBuilderV11(
710 base_url, project_id)
711 image_builder = nova.api.openstack.views.images.ViewBuilderV11(
712 base_url, project_id)
713 addresses_builder = nova.api.openstack.views.addresses.ViewBuilderV11()
714 builder = nova.api.openstack.views.servers.ViewBuilderV11(
715 addresses_builder, flavor_builder, image_builder,
716 base_url, project_id)
717 return builder.build_list(instances, is_detail=is_detail, **params)
718
695 def _action_change_password(self, input_dict, req, id):719 def _action_change_password(self, input_dict, req, id):
696 context = req.environ['nova.context']720 context = req.environ['nova.context']
697 if (not 'changePassword' in input_dict721 if (not 'changePassword' in input_dict
@@ -939,18 +963,22 @@
939 'security_group')963 'security_group')
940 group_elem.set('name', group['name'])964 group_elem.set('name', group['name'])
941965
942 for link in server_dict.get('links', []):966 self._populate_links(server_elem, server_dict.get('links', []))
943 elem = etree.SubElement(server_elem,967
968 def _populate_links(self, parent, links):
969 for link in links:
970 elem = etree.SubElement(parent,
944 '{%s}link' % xmlutil.XMLNS_ATOM)971 '{%s}link' % xmlutil.XMLNS_ATOM)
945 elem.set('rel', link['rel'])972 elem.set('rel', link['rel'])
946 elem.set('href', link['href'])973 elem.set('href', link['href'])
947 return server_elem
948974
949 def index(self, servers_dict):975 def index(self, servers_dict):
950 servers = etree.Element('servers', nsmap=self.NSMAP)976 servers = etree.Element('servers', nsmap=self.NSMAP)
951 for server_dict in servers_dict['servers']:977 for server_dict in servers_dict['servers']:
952 server = etree.SubElement(servers, 'server')978 server = etree.SubElement(servers, 'server')
953 self._populate_server(server, server_dict, False)979 self._populate_server(server, server_dict, False)
980
981 self._populate_links(servers, servers_dict.get('servers_links', []))
954 return self._to_xml(servers)982 return self._to_xml(servers)
955983
956 def detail(self, servers_dict):984 def detail(self, servers_dict):
957985
=== modified file 'nova/api/openstack/views/servers.py'
--- nova/api/openstack/views/servers.py 2011-09-09 13:48:38 +0000
+++ nova/api/openstack/views/servers.py 2011-09-20 18:47:17 +0000
@@ -40,7 +40,7 @@
40 def __init__(self, addresses_builder):40 def __init__(self, addresses_builder):
41 self.addresses_builder = addresses_builder41 self.addresses_builder = addresses_builder
4242
43 def build(self, inst, is_detail):43 def build(self, inst, is_detail=False):
44 """Return a dict that represenst a server."""44 """Return a dict that represenst a server."""
45 if inst.get('_is_precooked', False):45 if inst.get('_is_precooked', False):
46 server = dict(server=inst)46 server = dict(server=inst)
@@ -54,6 +54,16 @@
5454
55 return server55 return server
5656
57 def build_list(self, server_objs, is_detail=False, **kwargs):
58 limit = kwargs.get('limit', None)
59 servers = []
60 servers_links = []
61
62 for server_obj in server_objs:
63 servers.append(self.build(server_obj, is_detail)['server'])
64
65 return dict(servers=servers)
66
57 def _build_simple(self, inst):67 def _build_simple(self, inst):
58 """Return a simple model of a server."""68 """Return a simple model of a server."""
59 return dict(server=dict(id=inst['id'], name=inst['display_name']))69 return dict(server=dict(id=inst['id'], name=inst['display_name']))
@@ -205,6 +215,31 @@
205215
206 response["links"] = links216 response["links"] = links
207217
218 def build_list(self, server_objs, is_detail=False, **kwargs):
219 limit = kwargs.get('limit', None)
220 servers = []
221 servers_links = []
222
223 for server_obj in server_objs:
224 servers.append(self.build(server_obj, is_detail)['server'])
225
226 if (len(servers) and limit) and (limit == len(servers)):
227 next_link = self.generate_next_link(servers[-1]['id'],
228 kwargs, is_detail)
229 servers_links = [dict(rel='next', href=next_link)]
230
231 reval = dict(servers=servers)
232 if len(servers_links) > 0:
233 reval['servers_links'] = servers_links
234 return reval
235
236 def generate_next_link(self, server_id, params, is_detail=False):
237 """ Return an href string with proper limit and marker params"""
238 params['marker'] = server_id
239 return "%s?%s" % (
240 os.path.join(self.base_url, self.project_id, "servers"),
241 common.dict_to_query_str(params))
242
208 def generate_href(self, server_id):243 def generate_href(self, server_id):
209 """Create an url that refers to a specific server id."""244 """Create an url that refers to a specific server id."""
210 return os.path.join(self.base_url, self.project_id,245 return os.path.join(self.base_url, self.project_id,
211246
=== modified file 'nova/tests/api/openstack/test_servers.py'
--- nova/tests/api/openstack/test_servers.py 2011-09-19 20:16:49 +0000
+++ nova/tests/api/openstack/test_servers.py 2011-09-20 18:47:17 +0000
@@ -20,6 +20,7 @@
20import datetime20import datetime
21import json21import json
22import unittest22import unittest
23import urlparse
23from lxml import etree24from lxml import etree
24from xml.dom import minidom25from xml.dom import minidom
2526
@@ -1152,6 +1153,67 @@
1152 self.assertEqual(res.status_int, 400)1153 self.assertEqual(res.status_int, 400)
1153 self.assertTrue('limit' in res.body)1154 self.assertTrue('limit' in res.body)
11541155
1156 def test_get_servers_with_limit_v1_1(self):
1157 req = webob.Request.blank('/v1.1/fake/servers?limit=3')
1158 res = req.get_response(fakes.wsgi_app())
1159 servers = json.loads(res.body)['servers']
1160 servers_links = json.loads(res.body)['servers_links']
1161 self.assertEqual([s['id'] for s in servers], [0, 1, 2])
1162 self.assertEqual(servers_links[0]['rel'], 'next')
1163
1164 href_parts = urlparse.urlparse(servers_links[0]['href'])
1165 self.assertEqual('/v1.1/fake/servers', href_parts.path)
1166 params = urlparse.parse_qs(href_parts.query)
1167 self.assertDictMatch({'limit': ['3'], 'marker': ['2']}, params)
1168
1169 req = webob.Request.blank('/v1.1/fake/servers?limit=aaa')
1170 res = req.get_response(fakes.wsgi_app())
1171 self.assertEqual(res.status_int, 400)
1172 self.assertTrue('limit' in res.body)
1173
1174 def test_get_server_details_with_limit_v1_1(self):
1175 req = webob.Request.blank('/v1.1/fake/servers/detail?limit=3')
1176 res = req.get_response(fakes.wsgi_app())
1177 servers = json.loads(res.body)['servers']
1178 servers_links = json.loads(res.body)['servers_links']
1179 self.assertEqual([s['id'] for s in servers], [0, 1, 2])
1180 self.assertEqual(servers_links[0]['rel'], 'next')
1181
1182 href_parts = urlparse.urlparse(servers_links[0]['href'])
1183 self.assertEqual('/v1.1/fake/servers', href_parts.path)
1184 params = urlparse.parse_qs(href_parts.query)
1185 self.assertDictMatch({'limit': ['3'], 'marker': ['2']}, params)
1186
1187 req = webob.Request.blank('/v1.1/fake/servers/detail?limit=aaa')
1188 res = req.get_response(fakes.wsgi_app())
1189 self.assertEqual(res.status_int, 400)
1190 self.assertTrue('limit' in res.body)
1191
1192 def test_get_server_details_with_limit_and_other_params_v1_1(self):
1193 req = webob.Request.blank('/v1.1/fake/servers/detail?limit=3&blah=2:t')
1194 res = req.get_response(fakes.wsgi_app())
1195 servers = json.loads(res.body)['servers']
1196 servers_links = json.loads(res.body)['servers_links']
1197 self.assertEqual([s['id'] for s in servers], [0, 1, 2])
1198 self.assertEqual(servers_links[0]['rel'], 'next')
1199
1200 href_parts = urlparse.urlparse(servers_links[0]['href'])
1201 self.assertEqual('/v1.1/fake/servers', href_parts.path)
1202 params = urlparse.parse_qs(href_parts.query)
1203 self.assertDictMatch({'limit': ['3'], 'blah': ['2:t'],
1204 'marker': ['2']}, params)
1205
1206 req = webob.Request.blank('/v1.1/fake/servers/detail?limit=aaa')
1207 res = req.get_response(fakes.wsgi_app())
1208 self.assertEqual(res.status_int, 400)
1209 self.assertTrue('limit' in res.body)
1210
1211 def test_get_servers_with_too_big_limit_v1_1(self):
1212 req = webob.Request.blank('/v1.1/fake/servers?limit=30')
1213 res = req.get_response(fakes.wsgi_app())
1214 res_dict = json.loads(res.body)
1215 self.assertTrue('servers_links' not in res_dict)
1216
1155 def test_get_servers_with_offset(self):1217 def test_get_servers_with_offset(self):
1156 req = webob.Request.blank('/v1.0/servers?offset=2')1218 req = webob.Request.blank('/v1.0/servers?offset=2')
1157 res = req.get_response(fakes.wsgi_app())1219 res = req.get_response(fakes.wsgi_app())
@@ -1952,7 +2014,6 @@
1952 req.headers["content-type"] = "application/json"2014 req.headers["content-type"] = "application/json"
19532015
1954 res = req.get_response(fakes.wsgi_app())2016 res = req.get_response(fakes.wsgi_app())
1955 print res
1956 self.assertEqual(res.status_int, 202)2017 self.assertEqual(res.status_int, 202)
1957 server = json.loads(res.body)['server']2018 server = json.loads(res.body)['server']
1958 self.assertEqual(1, server['id'])2019 self.assertEqual(1, server['id'])
@@ -2480,6 +2541,7 @@
2480 }2541 }
2481 req = webob.Request.blank('/v1.1/fake/servers/detail')2542 req = webob.Request.blank('/v1.1/fake/servers/detail')
2482 res = req.get_response(fakes.wsgi_app())2543 res = req.get_response(fakes.wsgi_app())
2544 print res.body
2483 res_dict = json.loads(res.body)2545 res_dict = json.loads(res.body)
24842546
2485 for i, s in enumerate(res_dict['servers']):2547 for i, s in enumerate(res_dict['servers']):
@@ -4181,6 +4243,7 @@
41814243
4182 TIMESTAMP = "2010-10-11T10:30:22Z"4244 TIMESTAMP = "2010-10-11T10:30:22Z"
4183 SERVER_HREF = 'http://localhost/v1.1/servers/123'4245 SERVER_HREF = 'http://localhost/v1.1/servers/123'
4246 SERVER_NEXT = 'http://localhost/v1.1/servers?limit=%s&marker=%s'
4184 SERVER_BOOKMARK = 'http://localhost/servers/123'4247 SERVER_BOOKMARK = 'http://localhost/servers/123'
4185 IMAGE_BOOKMARK = 'http://localhost/images/5'4248 IMAGE_BOOKMARK = 'http://localhost/images/5'
4186 FLAVOR_BOOKMARK = 'http://localhost/flavors/1'4249 FLAVOR_BOOKMARK = 'http://localhost/flavors/1'
@@ -4599,6 +4662,74 @@
4599 for key, value in link.items():4662 for key, value in link.items():
4600 self.assertEqual(link_nodes[i].get(key), value)4663 self.assertEqual(link_nodes[i].get(key), value)
46014664
4665 def test_index_with_servers_links(self):
4666 serializer = servers.ServerXMLSerializer()
4667
4668 expected_server_href = 'http://localhost/v1.1/servers/1'
4669 expected_server_next = self.SERVER_NEXT % (2, 2)
4670 expected_server_bookmark = 'http://localhost/servers/1'
4671 expected_server_href_2 = 'http://localhost/v1.1/servers/2'
4672 expected_server_bookmark_2 = 'http://localhost/servers/2'
4673 fixture = {"servers": [
4674 {
4675 "id": 1,
4676 "name": "test_server",
4677 'links': [
4678 {
4679 'href': expected_server_href,
4680 'rel': 'self',
4681 },
4682 {
4683 'href': expected_server_bookmark,
4684 'rel': 'bookmark',
4685 },
4686 ],
4687 },
4688 {
4689 "id": 2,
4690 "name": "test_server_2",
4691 'links': [
4692 {
4693 'href': expected_server_href_2,
4694 'rel': 'self',
4695 },
4696 {
4697 'href': expected_server_bookmark_2,
4698 'rel': 'bookmark',
4699 },
4700 ],
4701 },
4702 ],
4703 "servers_links": [
4704 {
4705 'rel': 'next',
4706 'href': expected_server_next,
4707 },
4708 ]}
4709
4710 output = serializer.serialize(fixture, 'index')
4711 print output
4712 root = etree.XML(output)
4713 xmlutil.validate_schema(root, 'servers_index')
4714 server_elems = root.findall('{0}server'.format(NS))
4715 self.assertEqual(len(server_elems), 2)
4716 for i, server_elem in enumerate(server_elems):
4717 server_dict = fixture['servers'][i]
4718 for key in ['name', 'id']:
4719 self.assertEqual(server_elem.get(key), str(server_dict[key]))
4720
4721 link_nodes = server_elem.findall('{0}link'.format(ATOMNS))
4722 self.assertEqual(len(link_nodes), 2)
4723 for i, link in enumerate(server_dict['links']):
4724 for key, value in link.items():
4725 self.assertEqual(link_nodes[i].get(key), value)
4726
4727 # Check servers_links
4728 servers_links = root.findall('{0}link'.format(ATOMNS))
4729 for i, link in enumerate(fixture['servers_links']):
4730 for key, value in link.items():
4731 self.assertEqual(servers_links[i].get(key), value)
4732
4602 def test_detail(self):4733 def test_detail(self):
4603 serializer = servers.ServerXMLSerializer()4734 serializer = servers.ServerXMLSerializer()
46044735