From 11698a131fe6b99bfd91a977a975b07bcd4c2b2b Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Thu, 17 Mar 2011 03:21:09 -0400 Subject: Added mechanism for versioned controllers for openstack api versions 1.0/1.1. Create servers in the 1.1 api now supports imageRef/flavorRef instead of imageId/flavorId. --- etc/api-paste.ini | 18 +++++--- nova/api/openstack/__init__.py | 29 ++++++++++--- nova/api/openstack/auth.py | 2 +- nova/api/openstack/common.py | 1 + nova/api/openstack/servers.py | 64 +++++++++++++++++++++++----- nova/api/openstack/views/addresses.py | 16 +------ nova/api/openstack/views/flavors.py | 18 +------- nova/api/openstack/views/images.py | 18 +------- nova/api/openstack/views/servers.py | 49 ++++++++------------- nova/tests/api/openstack/fakes.py | 18 +++++--- nova/tests/api/openstack/test_auth.py | 6 +-- nova/tests/api/openstack/test_servers.py | 73 +++++++++++++++++++++++++++++--- 12 files changed, 191 insertions(+), 121 deletions(-) diff --git a/etc/api-paste.ini b/etc/api-paste.ini index a4483d3f8..2b395aa0c 100644 --- a/etc/api-paste.ini +++ b/etc/api-paste.ini @@ -67,11 +67,14 @@ paste.app_factory = nova.api.ec2.metadatarequesthandler:MetadataRequestHandler.f [composite:osapi] use = egg:Paste#urlmap /: osversions -/v1.0: openstackapi -/v1.1: openstackapi +/v1.0: openstackapi10 +/v1.1: openstackapi11 -[pipeline:openstackapi] -pipeline = faultwrap auth ratelimit osapiapp +[pipeline:openstackapi10] +pipeline = faultwrap auth ratelimit osapiapp10 + +[pipeline:openstackapi11] +pipeline = faultwrap auth ratelimit osapiapp11 [filter:faultwrap] paste.filter_factory = nova.api.openstack:FaultWrapper.factory @@ -82,8 +85,11 @@ paste.filter_factory = nova.api.openstack.auth:AuthMiddleware.factory [filter:ratelimit] paste.filter_factory = nova.api.openstack.ratelimiting:RateLimitingMiddleware.factory -[app:osapiapp] -paste.app_factory = nova.api.openstack:APIRouter.factory +[app:osapiapp10] +paste.app_factory = nova.api.openstack:APIRouterV10.factory + +[app:osapiapp11] +paste.app_factory = nova.api.openstack:APIRouterV11.factory [pipeline:osversions] pipeline = faultwrap osversionapp diff --git a/nova/api/openstack/__init__.py b/nova/api/openstack/__init__.py index 0244bc93c..0b50d17d0 100644 --- a/nova/api/openstack/__init__.py +++ b/nova/api/openstack/__init__.py @@ -71,9 +71,14 @@ class APIRouter(wsgi.Router): return cls() def __init__(self): + self.server_members = {} mapper = routes.Mapper() + self._setup_routes(mapper) + super(APIRouter, self).__init__(mapper) - server_members = {'action': 'POST'} + def _setup_routes(self, mapper): + server_members = self.server_members + server_members['action'] = 'POST' if FLAGS.allow_admin_api: LOG.debug(_("Including admin operations in API.")) @@ -98,10 +103,6 @@ class APIRouter(wsgi.Router): controller=accounts.Controller(), collection={'detail': 'GET'}) - mapper.resource("server", "servers", controller=servers.Controller(), - collection={'detail': 'GET'}, - member=server_members) - mapper.resource("backup_schedule", "backup_schedule", controller=backup_schedules.Controller(), parent_resource=dict(member_name='server', @@ -120,7 +121,23 @@ class APIRouter(wsgi.Router): collection={'detail': 'GET'}, controller=shared_ip_groups.Controller()) - super(APIRouter, self).__init__(mapper) + +class APIRouterV10(APIRouter): + def _setup_routes(self, mapper): + APIRouter._setup_routes(self, mapper) + mapper.resource("server", "servers", + controller=servers.ControllerV10(), + collection={'detail': 'GET'}, + member=self.server_members) + + +class APIRouterV11(APIRouter): + def _setup_routes(self, mapper): + APIRouter._setup_routes(self, mapper) + mapper.resource("server", "servers", + controller=servers.ControllerV11(), + collection={'detail': 'GET'}, + member=self.server_members) class Versions(wsgi.Application): diff --git a/nova/api/openstack/auth.py b/nova/api/openstack/auth.py index 7ae285019..6f1cf5e63 100644 --- a/nova/api/openstack/auth.py +++ b/nova/api/openstack/auth.py @@ -70,7 +70,7 @@ class AuthMiddleware(wsgi.Middleware): req.environ['nova.context'] = context.RequestContext(user, account) version = req.path.split('/')[1].replace('v', '') - req.environ['nova.api.openstack.version'] = version + req.environ['api.version'] = version return self.application def has_authentication(self, req): diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index d94969ff5..d6679de01 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -75,5 +75,6 @@ def get_image_id_from_image_hash(image_service, context, image_hash): return image_id raise exception.NotFound(image_hash) + def get_api_version(req): return req.environ.get('api.version') diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index dc62882eb..9ce0caa46 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -27,8 +27,9 @@ from nova import wsgi from nova import utils from nova.api.openstack import common from nova.api.openstack import faults -from nova.api.openstack.views import servers as servers_views -from nova.api.openstack.views import addresses as addresses_views +import nova.api.openstack.views.addresses +import nova.api.openstack.views.flavors +import nova.api.openstack.views.servers from nova.auth import manager as auth_manager from nova.compute import instance_types from nova.compute import power_state @@ -57,7 +58,7 @@ class Controller(wsgi.Controller): def ips(self, req, id): try: instance = self.compute_api.get(req.environ['nova.context'], id) - builder = addresses_views.get_view_builder(req) + builder = self._get_addresses_view_builder(req) return builder.build(instance) except exception.NotFound: return faults.Fault(exc.HTTPNotFound()) @@ -77,7 +78,7 @@ class Controller(wsgi.Controller): """ instance_list = self.compute_api.get_all(req.environ['nova.context']) limited_list = common.limited(instance_list, req) - builder = servers_views.get_view_builder(req) + builder = self._get_view_builder(req) servers = [builder.build(inst, is_detail)['server'] for inst in limited_list] return dict(servers=servers) @@ -86,7 +87,7 @@ class Controller(wsgi.Controller): """ Returns server details by server id """ try: instance = self.compute_api.get(req.environ['nova.context'], id) - builder = servers_views.get_view_builder(req) + builder = self._get_view_builder(req) return builder.build(instance, is_detail=True) except exception.NotFound: return faults.Fault(exc.HTTPNotFound()) @@ -111,8 +112,9 @@ class Controller(wsgi.Controller): raise exception.NotFound(_("No keypairs defined")) key_pair = key_pairs[0] + requested_image_id = self._image_id_from_req_data(env) image_id = common.get_image_id_from_image_hash(self._image_service, - context, env['server']['imageId']) + context, requested_image_id) kernel_id, ramdisk_id = self._get_kernel_ramdisk_from_image( req, image_id) @@ -126,9 +128,10 @@ class Controller(wsgi.Controller): for k, v in env['server']['metadata'].items(): metadata.append({'key': k, 'value': v}) - instances = self.compute_api.create( + flavor_id = self._flavor_id_from_req_data(env) + (inst,) = self.compute_api.create( context, - instance_types.get_by_flavor_id(env['server']['flavorId']), + instance_types.get_by_flavor_id(flavor_id), image_id, kernel_id=kernel_id, ramdisk_id=ramdisk_id, @@ -138,9 +141,11 @@ class Controller(wsgi.Controller): key_data=key_pair['public_key'], metadata=metadata, onset_files=env.get('onset_files', [])) + inst['instance_type'] = flavor_id + inst['image_id'] = requested_image_id - builder = servers_views.get_view_builder(req) - server = builder.build(instances[0], is_detail=False) + builder = self._get_view_builder(req) + server = builder.build(inst, is_detail=True) password = "%s%s" % (server['server']['name'][:4], utils.generate_password(12)) server['server']['adminPass'] = password @@ -437,3 +442,42 @@ class Controller(wsgi.Controller): _("Ramdisk not found for image %(image_id)s") % locals()) return kernel_id, ramdisk_id + + +class ControllerV10(Controller): + def _image_id_from_req_data(self, data): + return data['server']['imageId'] + + def _flavor_id_from_req_data(self, data): + return data['server']['flavorId'] + + def _get_view_builder(self, req): + addresses_builder = nova.api.openstack.views.addresses.ViewBuilderV10() + return nova.api.openstack.views.servers.ViewBuilderV10( + addresses_builder) + + def _get_addresses_view_builder(self, req): + return nova.api.openstack.views.addresses.ViewBuilderV10(req) + + +class ControllerV11(Controller): + def _image_id_from_req_data(self, data): + href = data['server']['imageRef'] + return href.split('/')[-1] + + def _flavor_id_from_req_data(self, data): + href = data['server']['flavorRef'] + return href.split('/')[-1] + + def _get_view_builder(self, req): + base_url = req.application_url + flavor_builder = nova.api.openstack.views.flavors.ViewBuilderV11( + base_url) + image_builder = nova.api.openstack.views.images.ViewBuilderV11( + base_url) + addresses_builder = nova.api.openstack.views.addresses.ViewBuilderV11() + return nova.api.openstack.views.servers.ViewBuilderV11( + addresses_builder, flavor_builder, image_builder) + + def _get_addresses_view_builder(self, req): + return nova.api.openstack.views.addresses.ViewBuilderV11(req) diff --git a/nova/api/openstack/views/addresses.py b/nova/api/openstack/views/addresses.py index 9d392aace..90c77855b 100644 --- a/nova/api/openstack/views/addresses.py +++ b/nova/api/openstack/views/addresses.py @@ -19,18 +19,6 @@ from nova import utils from nova.api.openstack import common -def get_view_builder(req): - ''' - A factory method that returns the correct builder based on the version of - the api requested. - ''' - version = common.get_api_version(req) - if version == '1.1': - return ViewBuilder_1_1() - else: - return ViewBuilder_1_0() - - class ViewBuilder(object): ''' Models a server addresses response as a python dictionary.''' @@ -38,14 +26,14 @@ class ViewBuilder(object): raise NotImplementedError() -class ViewBuilder_1_0(ViewBuilder): +class ViewBuilderV10(ViewBuilder): def build(self, inst): private_ips = utils.get_from_path(inst, 'fixed_ip/address') public_ips = utils.get_from_path(inst, 'fixed_ip/floating_ips/address') return dict(public=public_ips, private=private_ips) -class ViewBuilder_1_1(ViewBuilder): +class ViewBuilderV11(ViewBuilder): def build(self, inst): private_ips = utils.get_from_path(inst, 'fixed_ip/address') private_ips = [dict(version=4, addr=a) for a in private_ips] diff --git a/nova/api/openstack/views/flavors.py b/nova/api/openstack/views/flavors.py index aa3c2aeb2..18bd779c0 100644 --- a/nova/api/openstack/views/flavors.py +++ b/nova/api/openstack/views/flavors.py @@ -17,18 +17,6 @@ from nova.api.openstack import common -def get_view_builder(req): - ''' - A factory method that returns the correct builder based on the version of - the api requested. - ''' - version = common.get_api_version(req) - base_url = req.application_url - if version == '1.1': - return ViewBuilder_1_1(base_url) - else: - return ViewBuilder_1_0() - class ViewBuilder(object): def __init__(self): @@ -38,13 +26,9 @@ class ViewBuilder(object): raise NotImplementedError() -class ViewBuilder_1_1(ViewBuilder): +class ViewBuilderV11(ViewBuilder): def __init__(self, base_url): self.base_url = base_url def generate_href(self, flavor_id): return "%s/flavors/%s" % (self.base_url, flavor_id) - - -class ViewBuilder_1_0(ViewBuilder): - pass diff --git a/nova/api/openstack/views/images.py b/nova/api/openstack/views/images.py index 930b464b0..a6c6ad7d1 100644 --- a/nova/api/openstack/views/images.py +++ b/nova/api/openstack/views/images.py @@ -17,18 +17,6 @@ from nova.api.openstack import common -def get_view_builder(req): - ''' - A factory method that returns the correct builder based on the version of - the api requested. - ''' - version = common.get_api_version(req) - base_url = req.application_url - if version == '1.1': - return ViewBuilder_1_1(base_url) - else: - return ViewBuilder_1_0() - class ViewBuilder(object): def __init__(self): @@ -38,13 +26,9 @@ class ViewBuilder(object): raise NotImplementedError() -class ViewBuilder_1_1(ViewBuilder): +class ViewBuilderV11(ViewBuilder): def __init__(self, base_url): self.base_url = base_url def generate_href(self, image_id): return "%s/images/%s" % (self.base_url, image_id) - - -class ViewBuilder_1_0(ViewBuilder): - pass diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 261acfed0..8d47ac757 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -24,22 +24,6 @@ from nova.api.openstack.views import images as images_view from nova import utils -def get_view_builder(req): - ''' - A factory method that returns the correct builder based on the version of - the api requested. - ''' - version = common.get_api_version(req) - addresses_builder = addresses_view.get_view_builder(req) - if version == '1.1': - flavor_builder = flavors_view.get_view_builder(req) - image_builder = images_view.get_view_builder(req) - return ViewBuilder_1_1(addresses_builder, flavor_builder, - image_builder) - else: - return ViewBuilder_1_0(addresses_builder) - - class ViewBuilder(object): ''' Models a server response as a python dictionary. @@ -76,25 +60,20 @@ class ViewBuilder(object): power_state.FAILED: 'error'} inst_dict = {} - #mapped_keys = dict(status='state', imageId='image_id', - # flavorId='instance_type', name='display_name', id='id') - - mapped_keys = dict(status='state', name='display_name', id='id') - - for k, v in mapped_keys.iteritems(): - inst_dict[k] = inst[v] - - inst_dict['status'] = power_mapping[inst_dict['status']] + inst_dict['id'] = int(inst['id']) + inst_dict['name'] = inst['display_name'] + inst_dict['status'] = power_mapping[inst.get('state')] inst_dict['addresses'] = self.addresses_builder.build(inst) # Return the metadata as a dictionary metadata = {} - for item in inst['metadata']: - metadata[item['key']] = item['value'] + if 'metadata' in inst: + for item in inst['metadata']: + metadata[item['key']] = item['value'] inst_dict['metadata'] = metadata inst_dict['hostId'] = '' - if inst['host']: + if inst.get('host'): inst_dict['hostId'] = hashlib.sha224(inst['host']).hexdigest() self._build_image(inst_dict, inst) @@ -109,24 +88,30 @@ class ViewBuilder(object): raise NotImplementedError() -class ViewBuilder_1_0(ViewBuilder): +class ViewBuilderV10(ViewBuilder): def _build_image(self, response, inst): - response["imageId"] = inst["image_id"] + if inst.get('image_id') != None: + response['imageId'] = inst['image_id'] def _build_flavor(self, response, inst): - response["flavorId"] = inst["instance_type"] + if inst.get('instance_type') != None: + response['flavorId'] = inst['instance_type'] -class ViewBuilder_1_1(ViewBuilder): +class ViewBuilderV11(ViewBuilder): def __init__(self, addresses_builder, flavor_builder, image_builder): ViewBuilder.__init__(self, addresses_builder) self.flavor_builder = flavor_builder self.image_builder = image_builder def _build_image(self, response, inst): + if inst.get('image_id') == None: + return image_id = inst["image_id"] response["imageRef"] = self.image_builder.generate_href(image_id) def _build_flavor(self, response, inst): + if inst.get('instance_type') == None: + return flavor_id = inst["instance_type"] response["flavorRef"] = self.flavor_builder.generate_href(flavor_id) diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py index a3968b57b..370eb68cb 100644 --- a/nova/tests/api/openstack/fakes.py +++ b/nova/tests/api/openstack/fakes.py @@ -76,14 +76,18 @@ def fake_wsgi(self, req): return self.application -def wsgi_app(inner_application=None): - if not inner_application: - inner_application = openstack.APIRouter() +def wsgi_app(inner_app10=None, inner_app11=None): + if not inner_app10: + inner_app10 = openstack.APIRouterV10() + if not inner_app11: + inner_app11 = openstack.APIRouterV11() mapper = urlmap.URLMap() - api = openstack.FaultWrapper(auth.AuthMiddleware( - ratelimiting.RateLimitingMiddleware(inner_application))) - mapper['/v1.0'] = api - mapper['/v1.1'] = api + api10 = openstack.FaultWrapper(auth.AuthMiddleware( + ratelimiting.RateLimitingMiddleware(inner_app10))) + api11 = openstack.FaultWrapper(auth.AuthMiddleware( + ratelimiting.RateLimitingMiddleware(inner_app11))) + mapper['/v1.0'] = api10 + mapper['/v1.1'] = api11 mapper['/'] = openstack.FaultWrapper(openstack.Versions()) return mapper diff --git a/nova/tests/api/openstack/test_auth.py b/nova/tests/api/openstack/test_auth.py index aaaa4e415..b7f0dfbe5 100644 --- a/nova/tests/api/openstack/test_auth.py +++ b/nova/tests/api/openstack/test_auth.py @@ -82,8 +82,7 @@ class Test(test.TestCase): self.assertEqual(result.headers['X-Storage-Url'], "") token = result.headers['X-Auth-Token'] - self.stubs.Set(nova.api.openstack, 'APIRouter', - fakes.FakeRouter) + self.stubs.Set(nova.api.openstack, 'APIRouterV10', fakes.FakeRouter) req = webob.Request.blank('/v1.0/fake') req.headers['X-Auth-Token'] = token result = req.get_response(fakes.wsgi_app()) @@ -189,8 +188,7 @@ class TestLimiter(test.TestCase): self.assertEqual(len(result.headers['X-Auth-Token']), 40) token = result.headers['X-Auth-Token'] - self.stubs.Set(nova.api.openstack, 'APIRouter', - fakes.FakeRouter) + self.stubs.Set(nova.api.openstack, 'APIRouterV10', fakes.FakeRouter) req = webob.Request.blank('/v1.0/fake') req.method = 'POST' req.headers['X-Auth-Token'] = token diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 27d174fe9..0116bbed1 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -158,7 +158,7 @@ class ServersTest(test.TestCase): req = webob.Request.blank('/v1.0/servers/1') res = req.get_response(fakes.wsgi_app()) res_dict = json.loads(res.body) - self.assertEqual(res_dict['server']['id'], '1') + self.assertEqual(res_dict['server']['id'], 1) self.assertEqual(res_dict['server']['name'], 'server1') def test_get_server_by_id_with_addresses(self): @@ -169,7 +169,7 @@ class ServersTest(test.TestCase): req = webob.Request.blank('/v1.0/servers/1') res = req.get_response(fakes.wsgi_app()) res_dict = json.loads(res.body) - self.assertEqual(res_dict['server']['id'], '1') + self.assertEqual(res_dict['server']['id'], 1) self.assertEqual(res_dict['server']['name'], 'server1') addresses = res_dict['server']['addresses'] self.assertEqual(len(addresses["public"]), len(public)) @@ -177,7 +177,7 @@ class ServersTest(test.TestCase): self.assertEqual(len(addresses["private"]), 1) self.assertEqual(addresses["private"][0], private) - def test_get_server_by_id_with_addresses_v1_1(self): + def test_get_server_by_id_with_addresses_v11(self): private = "192.168.0.3" public = ["1.2.3.4"] new_return_server = return_server_with_addresses(private, public) @@ -186,7 +186,7 @@ class ServersTest(test.TestCase): req.environ['api.version'] = '1.1' res = req.get_response(fakes.wsgi_app()) res_dict = json.loads(res.body) - self.assertEqual(res_dict['server']['id'], '1') + self.assertEqual(res_dict['server']['id'], 1) self.assertEqual(res_dict['server']['name'], 'server1') addresses = res_dict['server']['addresses'] self.assertEqual(len(addresses["public"]), len(public)) @@ -273,13 +273,13 @@ class ServersTest(test.TestCase): "get_image_id_from_image_hash", image_id_from_hash) body = dict(server=dict( - name='server_test', imageId=2, flavorId=2, + name='server_test', imageId=3, flavorId=2, metadata={'hello': 'world', 'open': 'stack'}, personality={})) req = webob.Request.blank('/v1.0/servers') req.method = 'POST' req.body = json.dumps(body) - req.headers["Content-Type"] = "application/json" + req.headers["content-type"] = "application/json" res = req.get_response(fakes.wsgi_app()) @@ -287,8 +287,67 @@ class ServersTest(test.TestCase): self.assertEqual('serv', server['adminPass'][:4]) self.assertEqual(16, len(server['adminPass'])) self.assertEqual('server_test', server['name']) - self.assertEqual('1', server['id']) + self.assertEqual(1, server['id']) + self.assertEqual(2, server['flavorId']) + self.assertEqual(3, server['imageId']) + self.assertEqual(res.status_int, 200) + + def test_create_instance_v11(self): + def instance_create(context, inst): + return {'id': '1', 'display_name': 'server_test'} + + def server_update(context, id, params): + return instance_create(context, id) + + def fake_method(*args, **kwargs): + pass + + def project_get_network(context, user_id): + return dict(id='1', host='localhost') + + def queue_get_for(context, *args): + return 'network_topic' + + def kernel_ramdisk_mapping(*args, **kwargs): + return (1, 1) + + def image_id_from_hash(*args, **kwargs): + return 2 + + self.stubs.Set(nova.db.api, 'project_get_network', project_get_network) + self.stubs.Set(nova.db.api, 'instance_create', instance_create) + self.stubs.Set(nova.rpc, 'cast', fake_method) + self.stubs.Set(nova.rpc, 'call', fake_method) + self.stubs.Set(nova.db.api, 'instance_update', + server_update) + self.stubs.Set(nova.db.api, 'queue_get_for', queue_get_for) + self.stubs.Set(nova.network.manager.VlanManager, 'allocate_fixed_ip', + fake_method) + self.stubs.Set(nova.api.openstack.servers.Controller, + "_get_kernel_ramdisk_from_image", kernel_ramdisk_mapping) + self.stubs.Set(nova.api.openstack.common, + "get_image_id_from_image_hash", image_id_from_hash) + imageRef = 'http://localhost/v1.1/images/2' + flavorRef = 'http://localhost/v1.1/flavors/3' + body = dict(server=dict( + name='server_test', imageRef=imageRef, flavorRef=flavorRef, + metadata={'hello': 'world', 'open': 'stack'}, + personality={})) + req = webob.Request.blank('/v1.1/servers') + req.method = 'POST' + req.body = json.dumps(body) + req.headers["content-type"] = "application/json" + + res = req.get_response(fakes.wsgi_app()) + + server = json.loads(res.body)['server'] + self.assertEqual('serv', server['adminPass'][:4]) + self.assertEqual(16, len(server['adminPass'])) + self.assertEqual('server_test', server['name']) + self.assertEqual(1, server['id']) + self.assertEqual(flavorRef, server['flavorRef']) + self.assertEqual(imageRef, server['imageRef']) self.assertEqual(res.status_int, 200) def test_update_no_body(self): -- cgit From 05ca6e24d4a3cf64bbe371f1c9c74088110eba68 Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Thu, 17 Mar 2011 04:32:24 -0400 Subject: Setting the api verion in the request in the auth middle is no longer needed. Also, common.get_api_version is no longer needed. As Eric Day noted, having versioned controllers will make that unnecessary. --- nova/api/openstack/auth.py | 2 -- nova/api/openstack/common.py | 4 ---- 2 files changed, 6 deletions(-) diff --git a/nova/api/openstack/auth.py b/nova/api/openstack/auth.py index 6f1cf5e63..4c6b58eff 100644 --- a/nova/api/openstack/auth.py +++ b/nova/api/openstack/auth.py @@ -69,8 +69,6 @@ class AuthMiddleware(wsgi.Middleware): return faults.Fault(webob.exc.HTTPUnauthorized()) req.environ['nova.context'] = context.RequestContext(user, account) - version = req.path.split('/')[1].replace('v', '') - req.environ['api.version'] = version return self.application def has_authentication(self, req): diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index d6679de01..74ac21024 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -74,7 +74,3 @@ def get_image_id_from_image_hash(image_service, context, image_hash): if abs(hash(image_id)) == int(image_hash): return image_id raise exception.NotFound(image_hash) - - -def get_api_version(req): - return req.environ.get('api.version') -- cgit From e138e0836922ee0608feefbff5e4e5d03ad14197 Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Thu, 17 Mar 2011 16:02:37 -0400 Subject: Now returns a 400 for a create server request with invalid hrefs for imageRef/flavorRef values. Also added tests. --- nova/api/openstack/common.py | 12 +++++- nova/api/openstack/servers.py | 5 +-- nova/tests/api/openstack/test_servers.py | 64 +++++++++++++------------------- 3 files changed, 37 insertions(+), 44 deletions(-) diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index 74ac21024..b224cbfb4 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -15,9 +15,10 @@ # License for the specific language governing permissions and limitations # under the License. -import webob.exc - +import re from nova import exception +from webob import exc +import webob.exc def limited(items, request, max_limit=1000): @@ -74,3 +75,10 @@ def get_image_id_from_image_hash(image_service, context, image_hash): if abs(hash(image_id)) == int(image_hash): return image_id raise exception.NotFound(image_hash) + + +def get_id_from_href(href): + m = re.match(r'http.+/.+/(\d)+$', href) + if not m: + raise exc.HTTPBadRequest(_('could not parse id from href')) + return int(m.group(1)) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index f03225b55..6f25d10bd 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -512,7 +512,6 @@ class Controller(wsgi.Controller): return kernel_id, ramdisk_id - class ControllerV10(Controller): def _image_id_from_req_data(self, data): return data['server']['imageId'] @@ -532,11 +531,11 @@ class ControllerV10(Controller): class ControllerV11(Controller): def _image_id_from_req_data(self, data): href = data['server']['imageRef'] - return href.split('/')[-1] + return common.get_id_from_href(href) def _flavor_id_from_req_data(self, data): href = data['server']['flavorRef'] - return href.split('/')[-1] + return common.get_id_from_href(href) def _get_view_builder(self, req): base_url = req.application_url diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 846af5c3a..6e78db9da 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -239,7 +239,7 @@ class ServersTest(test.TestCase): servers = json.loads(res.body)['servers'] self.assertEqual([s['id'] for s in servers], [1, 2]) - def _test_create_instance_helper(self): + def _setup_for_create_instance(self): """Shared implementation for tests below that create instance""" def instance_create(context, inst): return {'id': '1', 'display_name': 'server_test'} @@ -276,6 +276,9 @@ class ServersTest(test.TestCase): self.stubs.Set(nova.api.openstack.common, "get_image_id_from_image_hash", image_id_from_hash) + def _test_create_instance_helper(self): + self._setup_for_create_instance() + body = dict(server=dict( name='server_test', imageId=3, flavorId=2, metadata={'hello': 'world', 'open': 'stack'}, @@ -296,41 +299,15 @@ class ServersTest(test.TestCase): self.assertEqual(3, server['imageId']) self.assertEqual(res.status_int, 200) - def test_create_instance_v11(self): - def instance_create(context, inst): - return {'id': '1', 'display_name': 'server_test'} - - def server_update(context, id, params): - return instance_create(context, id) - - def fake_method(*args, **kwargs): - pass - - def project_get_network(context, user_id): - return dict(id='1', host='localhost') - - def queue_get_for(context, *args): - return 'network_topic' - - def kernel_ramdisk_mapping(*args, **kwargs): - return (1, 1) + def test_create_instance(self): + self._test_create_instance_helper() - def image_id_from_hash(*args, **kwargs): - return 2 + def test_create_instance_no_key_pair(self): + fakes.stub_out_key_pair_funcs(self.stubs, have_key_pair=False) + self._test_create_instance_helper() - self.stubs.Set(nova.db.api, 'project_get_network', project_get_network) - self.stubs.Set(nova.db.api, 'instance_create', instance_create) - self.stubs.Set(nova.rpc, 'cast', fake_method) - self.stubs.Set(nova.rpc, 'call', fake_method) - self.stubs.Set(nova.db.api, 'instance_update', - server_update) - self.stubs.Set(nova.db.api, 'queue_get_for', queue_get_for) - self.stubs.Set(nova.network.manager.VlanManager, 'allocate_fixed_ip', - fake_method) - self.stubs.Set(nova.api.openstack.servers.Controller, - "_get_kernel_ramdisk_from_image", kernel_ramdisk_mapping) - self.stubs.Set(nova.api.openstack.common, - "get_image_id_from_image_hash", image_id_from_hash) + def test_create_instance_v11(self): + self._setup_for_create_instance() imageRef = 'http://localhost/v1.1/images/2' flavorRef = 'http://localhost/v1.1/flavors/3' @@ -354,12 +331,21 @@ class ServersTest(test.TestCase): self.assertEqual(imageRef, server['imageRef']) self.assertEqual(res.status_int, 200) - def test_create_instance(self): - self._test_create_instance_helper() + def test_create_instance_v11_bad_href(self): + self._setup_for_create_instance() - def test_create_instance_no_key_pair(self): - fakes.stub_out_key_pair_funcs(self.stubs, have_key_pair=False) - self._test_create_instance_helper() + imageRef = 'http://localhost/v1.1/images/asdf' + flavorRef = 'http://localhost/v1.1/flavors/3' + body = dict(server=dict( + name='server_test', imageRef=imageRef, flavorRef=flavorRef, + metadata={'hello': 'world', 'open': 'stack'}, + personality={})) + req = webob.Request.blank('/v1.1/servers') + req.method = 'POST' + req.body = json.dumps(body) + req.headers["content-type"] = "application/json" + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 400) def test_update_no_body(self): req = webob.Request.blank('/v1.0/servers/1') -- cgit From 85f50cf496e2c193ddc715f3019b4a4769ab5bd9 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Mon, 21 Mar 2011 15:14:24 -0400 Subject: pep8; various fixes --- nova/api/openstack/servers.py | 1 + nova/api/openstack/views/servers.py | 5 ++--- nova/tests/api/openstack/test_servers.py | 17 +++++++++++++---- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index e3141934b..dafc096ba 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -513,6 +513,7 @@ class Controller(wsgi.Controller): return kernel_id, ramdisk_id + class ControllerV10(Controller): def _image_id_from_req_data(self, data): return data['server']['imageId'] diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 8d47ac757..078d5d484 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -67,9 +67,8 @@ class ViewBuilder(object): # Return the metadata as a dictionary metadata = {} - if 'metadata' in inst: - for item in inst['metadata']: - metadata[item['key']] = item['value'] + for item in inst.get('metadata', []): + metadata[item['key']] = item['value'] inst_dict['metadata'] = metadata inst_dict['hostId'] = '' diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 6e78db9da..a9e76b244 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -311,10 +311,19 @@ class ServersTest(test.TestCase): imageRef = 'http://localhost/v1.1/images/2' flavorRef = 'http://localhost/v1.1/flavors/3' - body = dict(server=dict( - name='server_test', imageRef=imageRef, flavorRef=flavorRef, - metadata={'hello': 'world', 'open': 'stack'}, - personality={})) + body = { + 'server': { + 'name': 'server_test', + 'imageRef': imageRef, + 'flavorRef': flavorRef, + 'metadata': { + 'hello': 'world', + 'open': 'stack', + }, + 'personality': {}, + }, + } + req = webob.Request.blank('/v1.1/servers') req.method = 'POST' req.body = json.dumps(body) -- cgit From 4e33ab9fc16d580fbcf57da8e6e2228ad27cc1af Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Tue, 22 Mar 2011 11:46:00 -0400 Subject: Adding more docstrings. image_id and instance_type fields of an instance will always exist, so no reason to check if keys exist. --- nova/api/openstack/__init__.py | 4 ++++ nova/api/openstack/views/servers.py | 25 ++++++++++++------------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/nova/api/openstack/__init__.py b/nova/api/openstack/__init__.py index 35b04f863..21d354f1c 100644 --- a/nova/api/openstack/__init__.py +++ b/nova/api/openstack/__init__.py @@ -131,6 +131,8 @@ class APIRouter(wsgi.Router): class APIRouterV10(APIRouter): + ''' Defines routes specific to OpenStack API V1.0 ''' + def _setup_routes(self, mapper): APIRouter._setup_routes(self, mapper) mapper.resource("server", "servers", @@ -140,6 +142,8 @@ class APIRouterV10(APIRouter): class APIRouterV11(APIRouter): + ''' Defines routes specific to OpenStack API V1.1 ''' + def _setup_routes(self, mapper): APIRouter._setup_routes(self, mapper) mapper.resource("server", "servers", diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 078d5d484..3100c46b5 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -34,19 +34,18 @@ class ViewBuilder(object): self.addresses_builder = addresses_builder def build(self, inst, is_detail): - """ - Coerces into dictionary format, mapping everything to - Rackspace-like attributes for return - """ + ''' Returns a dict that represenst a server ''' if is_detail: return self._build_detail(inst) else: return self._build_simple(inst) def _build_simple(self, inst): - return dict(server=dict(id=inst['id'], name=inst['display_name'])) + ''' Returns a simple model of a server ''' + return dict(server=dict(id=inst['id'], name=inst['display_name'])) def _build_detail(self, inst): + ''' Returns a detailed model of a server ''' power_mapping = { None: 'build', power_state.NOSTATE: 'build', @@ -81,36 +80,36 @@ class ViewBuilder(object): return dict(server=inst_dict) def _build_image(self, response, inst): + ''' Returns the image sub-resource of a server ''' raise NotImplementedError() def _build_flavor(self, response, inst): + ''' Returns the flavor sub-resource of a server ''' raise NotImplementedError() class ViewBuilderV10(ViewBuilder): + ''' Models an Openstack API V1.0 server response ''' + def _build_image(self, response, inst): - if inst.get('image_id') != None: - response['imageId'] = inst['image_id'] + response['imageId'] = inst['image_id'] def _build_flavor(self, response, inst): - if inst.get('instance_type') != None: - response['flavorId'] = inst['instance_type'] + response['flavorId'] = inst['instance_type'] class ViewBuilderV11(ViewBuilder): + ''' Models an Openstack API V1.0 server response ''' + def __init__(self, addresses_builder, flavor_builder, image_builder): ViewBuilder.__init__(self, addresses_builder) self.flavor_builder = flavor_builder self.image_builder = image_builder def _build_image(self, response, inst): - if inst.get('image_id') == None: - return image_id = inst["image_id"] response["imageRef"] = self.image_builder.generate_href(image_id) def _build_flavor(self, response, inst): - if inst.get('instance_type') == None: - return flavor_id = inst["instance_type"] response["flavorRef"] = self.flavor_builder.generate_href(flavor_id) -- cgit From 493e87976b7eb273f4115d46c91ad73671abb796 Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Tue, 22 Mar 2011 13:18:08 -0400 Subject: Now using urlparse to parse a url to grab id out of it. --- nova/api/openstack/common.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index b224cbfb4..99fba8fef 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -17,6 +17,7 @@ import re from nova import exception +from urlparse import urlparse from webob import exc import webob.exc @@ -78,7 +79,7 @@ def get_image_id_from_image_hash(image_service, context, image_hash): def get_id_from_href(href): - m = re.match(r'http.+/.+/(\d)+$', href) - if not m: + try: + return int(urlparse(href).path.split('/')[-1]) + except: raise exc.HTTPBadRequest(_('could not parse id from href')) - return int(m.group(1)) -- cgit From ca37b31d64f9c5cf32ca7e6015176ef36e702dce Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Tue, 22 Mar 2011 16:04:27 -0400 Subject: Updating doc strings in accordance with PEP 257. Fixing order of imports in common.py. --- nova/api/openstack/__init__.py | 4 ++-- nova/api/openstack/common.py | 16 +++++++++++----- nova/api/openstack/views/servers.py | 22 ++++++++++++---------- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/nova/api/openstack/__init__.py b/nova/api/openstack/__init__.py index 21d354f1c..5f9648210 100644 --- a/nova/api/openstack/__init__.py +++ b/nova/api/openstack/__init__.py @@ -131,7 +131,7 @@ class APIRouter(wsgi.Router): class APIRouterV10(APIRouter): - ''' Defines routes specific to OpenStack API V1.0 ''' + """Define routes specific to OpenStack API V1.0.""" def _setup_routes(self, mapper): APIRouter._setup_routes(self, mapper) @@ -142,7 +142,7 @@ class APIRouterV10(APIRouter): class APIRouterV11(APIRouter): - ''' Defines routes specific to OpenStack API V1.1 ''' + """Define routes specific to OpenStack API V1.1.""" def _setup_routes(self, mapper): APIRouter._setup_routes(self, mapper) diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index 99fba8fef..21ceec45e 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -15,11 +15,11 @@ # License for the specific language governing permissions and limitations # under the License. -import re -from nova import exception from urlparse import urlparse -from webob import exc -import webob.exc + +import webob + +from nova import exception def limited(items, request, max_limit=1000): @@ -79,7 +79,13 @@ def get_image_id_from_image_hash(image_service, context, image_hash): def get_id_from_href(href): + """Return the id portion of a url. + + Given: http://www.foo.com/bar/123?q=4 + Returns: 4 + + """ try: return int(urlparse(href).path.split('/')[-1]) except: - raise exc.HTTPBadRequest(_('could not parse id from href')) + raise webob.exc.HTTPBadRequest(_('could not parse id from href')) diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 3100c46b5..fad361bd4 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -25,27 +25,29 @@ from nova import utils class ViewBuilder(object): - ''' - Models a server response as a python dictionary. + """Model a server response as a python dictionary. + + Public methods: build Abstract methods: _build_image, _build_flavor - ''' + + """ def __init__(self, addresses_builder): self.addresses_builder = addresses_builder def build(self, inst, is_detail): - ''' Returns a dict that represenst a server ''' + """Return a dict that represenst a server.""" if is_detail: return self._build_detail(inst) else: return self._build_simple(inst) def _build_simple(self, inst): - ''' Returns a simple model of a server ''' + """Return a simple model of a server.""" return dict(server=dict(id=inst['id'], name=inst['display_name'])) def _build_detail(self, inst): - ''' Returns a detailed model of a server ''' + """Returns a detailed model of a server.""" power_mapping = { None: 'build', power_state.NOSTATE: 'build', @@ -80,16 +82,16 @@ class ViewBuilder(object): return dict(server=inst_dict) def _build_image(self, response, inst): - ''' Returns the image sub-resource of a server ''' + """Return the image sub-resource of a server.""" raise NotImplementedError() def _build_flavor(self, response, inst): - ''' Returns the flavor sub-resource of a server ''' + """Return the flavor sub-resource of a server.""" raise NotImplementedError() class ViewBuilderV10(ViewBuilder): - ''' Models an Openstack API V1.0 server response ''' + """Model an Openstack API V1.0 server response.""" def _build_image(self, response, inst): response['imageId'] = inst['image_id'] @@ -99,7 +101,7 @@ class ViewBuilderV10(ViewBuilder): class ViewBuilderV11(ViewBuilder): - ''' Models an Openstack API V1.0 server response ''' + """Model an Openstack API V1.0 server response.""" def __init__(self, addresses_builder, flavor_builder, image_builder): ViewBuilder.__init__(self, addresses_builder) -- cgit From ea92a88b727814698dbc4ebf5dc705677d636445 Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Wed, 23 Mar 2011 14:05:21 -0400 Subject: Using super to call parent _setup_routes in APIRouter subclasses. --- nova/api/openstack/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/api/openstack/__init__.py b/nova/api/openstack/__init__.py index 5f9648210..e68110bc4 100644 --- a/nova/api/openstack/__init__.py +++ b/nova/api/openstack/__init__.py @@ -134,7 +134,7 @@ class APIRouterV10(APIRouter): """Define routes specific to OpenStack API V1.0.""" def _setup_routes(self, mapper): - APIRouter._setup_routes(self, mapper) + super(APIRouterV10, self)._setup_routes(mapper) mapper.resource("server", "servers", controller=servers.ControllerV10(), collection={'detail': 'GET'}, @@ -145,7 +145,7 @@ class APIRouterV11(APIRouter): """Define routes specific to OpenStack API V1.1.""" def _setup_routes(self, mapper): - APIRouter._setup_routes(self, mapper) + super(APIRouterV11, self)._setup_routes(mapper) mapper.resource("server", "servers", controller=servers.ControllerV11(), collection={'detail': 'GET'}, -- cgit From 0d677a9b63ed9b4612379494bf8a58af1c090331 Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Wed, 23 Mar 2011 16:51:30 -0400 Subject: Should not call super __init__ twice in APIRouter --- nova/api/openstack/__init__.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/nova/api/openstack/__init__.py b/nova/api/openstack/__init__.py index e68110bc4..143b1d2b2 100644 --- a/nova/api/openstack/__init__.py +++ b/nova/api/openstack/__init__.py @@ -127,8 +127,6 @@ class APIRouter(wsgi.Router): _limits = limits.LimitsController() mapper.resource("limit", "limits", controller=_limits) - super(APIRouter, self).__init__(mapper) - class APIRouterV10(APIRouter): """Define routes specific to OpenStack API V1.0.""" -- cgit From f52a2a8a440b303e5289815ab4f6c2d24bfdc59f Mon Sep 17 00:00:00 2001 From: Naveed Massjouni Date: Thu, 24 Mar 2011 01:41:38 -0400 Subject: Fixed the docstring for common.get_id_from_href --- nova/api/openstack/common.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index 21ceec45e..bff050347 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -79,10 +79,10 @@ def get_image_id_from_image_hash(image_service, context, image_hash): def get_id_from_href(href): - """Return the id portion of a url. + """Return the id portion of a url as an int. Given: http://www.foo.com/bar/123?q=4 - Returns: 4 + Returns: 123 """ try: -- cgit