From 3335a91c3c53513cc35e3f39a59975b33524950b Mon Sep 17 00:00:00 2001 From: Tushar Patil Date: Fri, 29 Jul 2011 18:45:51 -0700 Subject: Fixed review comments: Put parsing logic of network information in create_instance_helper module and refactored unit testcases as per the changed code. --- nova/api/openstack/contrib/createserverext.py | 213 +------------------------- nova/api/openstack/create_instance_helper.py | 83 +++++++++- 2 files changed, 84 insertions(+), 212 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/contrib/createserverext.py b/nova/api/openstack/contrib/createserverext.py index b5c06920c..79c017b42 100644 --- a/nova/api/openstack/contrib/createserverext.py +++ b/nova/api/openstack/contrib/createserverext.py @@ -13,220 +13,11 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License -from webob import exc - -from nova import exception -import nova.image -from nova import network -from nova import quota -from nova import rpc from nova.api.openstack import create_instance_helper as helper from nova.api.openstack import extensions -from nova.compute import instance_types from nova.api.openstack import servers from nova.api.openstack import wsgi -from nova.auth import manager as auth_manager - - -class CreateInstanceHelperEx(helper.CreateInstanceHelper): - def __init__(self, controller): - super(CreateInstanceHelperEx, self).__init__(controller) - - def create_instance(self, req, body, create_method): - """Creates a new server for the given user as per - the network information if it is provided - """ - if not body: - raise exc.HTTPUnprocessableEntity() - - if not 'server' in body: - raise exc.HTTPUnprocessableEntity() - - context = req.environ['nova.context'] - - password = self.controller._get_server_admin_password(body['server']) - - key_name = None - key_data = None - key_pairs = auth_manager.AuthManager.get_key_pairs(context) - if key_pairs: - key_pair = key_pairs[0] - key_name = key_pair['name'] - key_data = key_pair['public_key'] - - image_href = self.controller._image_ref_from_req_data(body) - try: - image_service, image_id = nova.image.get_image_service(image_href) - kernel_id, ramdisk_id = self._get_kernel_ramdisk_from_image( - req, image_id) - images = set([str(x['id']) for x in image_service.index(context)]) - assert str(image_id) in images - except Exception, e: - msg = _("Cannot find requested image %(image_href)s: %(e)s" % - locals()) - raise exc.HTTPBadRequest(explanation=msg) - - personality = body['server'].get('personality') - - injected_files = [] - if personality: - injected_files = self._get_injected_files(personality) - - requested_networks = body['server'].get('networks') - - if requested_networks is not None: - requested_networks = self._get_requested_networks( - requested_networks) - - flavor_id = self.controller._flavor_id_from_req_data(body) - - if not 'name' in body['server']: - msg = _("Server name is not defined") - raise exc.HTTPBadRequest(explanation=msg) - - zone_blob = body['server'].get('blob') - name = body['server']['name'] - self._validate_server_name(name) - name = name.strip() - - reservation_id = body['server'].get('reservation_id') - min_count = body['server'].get('min_count') - max_count = body['server'].get('max_count') - # min_count and max_count are optional. If they exist, they come - # in as strings. We want to default 'min_count' to 1, and default - # 'max_count' to be 'min_count'. - min_count = int(min_count) if min_count else 1 - max_count = int(max_count) if max_count else min_count - if min_count > max_count: - min_count = max_count - - try: - inst_type = \ - instance_types.get_instance_type_by_flavor_id(flavor_id) - extra_values = { - 'instance_type': inst_type, - 'image_ref': image_href, - 'password': password} - - return (extra_values, - create_method(context, - inst_type, - image_id, - kernel_id=kernel_id, - ramdisk_id=ramdisk_id, - display_name=name, - display_description=name, - key_name=key_name, - key_data=key_data, - metadata=body['server'].get('metadata', {}), - injected_files=injected_files, - admin_password=password, - zone_blob=zone_blob, - reservation_id=reservation_id, - min_count=min_count, - max_count=max_count, - requested_networks=requested_networks)) - except quota.QuotaError as error: - self._handle_quota_error(error) - except exception.ImageNotFound as error: - msg = _("Can not find requested image") - raise exc.HTTPBadRequest(explanation=msg) - except rpc.RemoteError as err: - msg = "%(err_type)s: %(err_msg)s" % \ - {'err_type': err.exc_type, 'err_msg': err.value} - raise exc.HTTPBadRequest(explanation=msg) - - # Let the caller deal with unhandled exceptions. - - def _validate_fixed_ip(self, value): - if not isinstance(value, basestring): - msg = _("Fixed IP is not a string or unicode") - raise exc.HTTPBadRequest(explanation=msg) - - if value.strip() == '': - msg = _("Fixed IP is an empty string") - raise exc.HTTPBadRequest(explanation=msg) - - def _get_requested_networks(self, requested_networks): - """ - Create a list of requested networks from the networks attribute - """ - networks = [] - for network in requested_networks: - try: - network_id = network['id'] - network_id = int(network_id) - #fixed IP address is optional - #if the fixed IP address is not provided then - #it will use one of the available IP address from the network - fixed_ip = network.get('fixed_ip', None) - if fixed_ip is not None: - self._validate_fixed_ip(fixed_ip) - # check if the network id is already present in the list, - # we don't want duplicate networks to be passed - # at the boot time - for id, ip in networks: - if id == network_id: - expl = _("Duplicate networks (%s) are not allowed")\ - % network_id - raise exc.HTTPBadRequest(explanation=expl) - - networks.append((network_id, fixed_ip)) - except KeyError as key: - expl = _('Bad network format: missing %s') % key - raise exc.HTTPBadRequest(explanation=expl) - except ValueError: - expl = _("Bad networks format: network id should " - "be integer (%s)") % network_id - raise exc.HTTPBadRequest(explanation=expl) - except TypeError: - expl = _('Bad networks format') - raise exc.HTTPBadRequest(explanation=expl) - - return networks - - -class CreateServerExtController(servers.ControllerV11): - """This is the controller for the extended version - of the create server OS V1.1 - """ - def __init__(self): - super(CreateServerExtController, self).__init__() - self.helper = CreateInstanceHelperEx(self) - - -class ServerXMLDeserializer(helper.ServerXMLDeserializer): - """ - Deserializer to handle xml-formatted server create requests. - - Handles networks element - """ - def _extract_server(self, node): - """Marshal the server attribute of a parsed request""" - server = super(ServerXMLDeserializer, self)._extract_server(node) - server_node = self.find_first_child_named(node, 'server') - networks = self._extract_networks(server_node) - if networks is not None: - server["networks"] = networks - return server - - def _extract_networks(self, server_node): - """Marshal the networks attribute of a parsed request""" - networks_node = \ - self.find_first_child_named(server_node, "networks") - if networks_node is None: - return None - networks = [] - for network_node in self.find_children_named(networks_node, - "network"): - item = {} - if network_node.hasAttribute("id"): - item["id"] = network_node.getAttribute("id") - if network_node.hasAttribute("fixed_ip"): - item["fixed_ip"] = network_node.getAttribute("fixed_ip") - networks.append(item) - return networks class Createserverext(extensions.ExtensionDescriptor): @@ -261,7 +52,7 @@ class Createserverext(extensions.ExtensionDescriptor): } body_deserializers = { - 'application/xml': ServerXMLDeserializer(), + 'application/xml': helper.ServerXMLDeserializer(), } serializer = wsgi.ResponseSerializer(body_serializers, @@ -269,7 +60,7 @@ class Createserverext(extensions.ExtensionDescriptor): deserializer = wsgi.RequestDeserializer(body_deserializers) res = extensions.ResourceExtension('os-create-server-ext', - controller=CreateServerExtController(), + controller=servers.ControllerV11(), deserializer=deserializer, serializer=serializer) resources.append(res) diff --git a/nova/api/openstack/create_instance_helper.py b/nova/api/openstack/create_instance_helper.py index 04fbeda73..c7cc88313 100644 --- a/nova/api/openstack/create_instance_helper.py +++ b/nova/api/openstack/create_instance_helper.py @@ -30,6 +30,7 @@ from nova import utils from nova.compute import instance_types from nova.api.openstack import wsgi +from nova.rpc.common import RemoteError LOG = logging.getLogger('nova.api.openstack.create_instance_helper') FLAGS = flags.FLAGS @@ -106,6 +107,12 @@ class CreateInstanceHelper(object): if personality: injected_files = self._get_injected_files(personality) + requested_networks = server_dict.get('networks') + + if requested_networks is not None: + requested_networks = self._get_requested_networks( + requested_networks) + try: flavor_id = self.controller._flavor_id_from_req_data(body) except ValueError as error: @@ -156,7 +163,8 @@ class CreateInstanceHelper(object): zone_blob=zone_blob, reservation_id=reservation_id, min_count=min_count, - max_count=max_count)) + max_count=max_count, + requested_networks=requested_networks)) except quota.QuotaError as error: self._handle_quota_error(error) except exception.ImageNotFound as error: @@ -165,6 +173,10 @@ class CreateInstanceHelper(object): except exception.FlavorNotFound as error: msg = _("Invalid flavorRef provided.") raise exc.HTTPBadRequest(explanation=msg) + except RemoteError as err: + msg = "%(err_type)s: %(err_msg)s" % \ + {'err_type': err.exc_type, 'err_msg': err.value} + raise exc.HTTPBadRequest(explanation=msg) # Let the caller deal with unhandled exceptions. def _handle_quota_error(self, error): @@ -286,6 +298,53 @@ class CreateInstanceHelper(object): raise exc.HTTPBadRequest(explanation=msg) return password + def _validate_fixed_ip(self, value): + if not isinstance(value, basestring): + msg = _("Fixed IP is not a string or unicode") + raise exc.HTTPBadRequest(explanation=msg) + + if value.strip() == '': + msg = _("Fixed IP is an empty string") + raise exc.HTTPBadRequest(explanation=msg) + + def _get_requested_networks(self, requested_networks): + """ + Create a list of requested networks from the networks attribute + """ + networks = [] + for network in requested_networks: + try: + network_id = network['id'] + network_id = int(network_id) + #fixed IP address is optional + #if the fixed IP address is not provided then + #it will use one of the available IP address from the network + fixed_ip = network.get('fixed_ip', None) + if fixed_ip is not None: + self._validate_fixed_ip(fixed_ip) + # check if the network id is already present in the list, + # we don't want duplicate networks to be passed + # at the boot time + for id, ip in networks: + if id == network_id: + expl = _("Duplicate networks (%s) are not allowed")\ + % network_id + raise exc.HTTPBadRequest(explanation=expl) + + networks.append((network_id, fixed_ip)) + except KeyError as key: + expl = _('Bad network format: missing %s') % key + raise exc.HTTPBadRequest(explanation=expl) + except ValueError: + expl = _("Bad networks format: network id should " + "be integer (%s)") % network_id + raise exc.HTTPBadRequest(explanation=expl) + except TypeError: + expl = _('Bad networks format') + raise exc.HTTPBadRequest(explanation=expl) + + return networks + class ServerXMLDeserializer(wsgi.MetadataXMLDeserializer): """ @@ -317,6 +376,10 @@ class ServerXMLDeserializer(wsgi.MetadataXMLDeserializer): server["personality"] = self._extract_personality(server_node) + networks = self._extract_networks(server_node) + if networks: + server["networks"] = networks + return server def _extract_personality(self, server_node): @@ -331,3 +394,21 @@ class ServerXMLDeserializer(wsgi.MetadataXMLDeserializer): item["contents"] = self.extract_text(file_node) personality.append(item) return personality + + def _extract_networks(self, server_node): + """Marshal the networks attribute of a parsed request""" + networks_node = \ + self.find_first_child_named(server_node, "networks") + if networks_node is None: + return None + networks = [] + if networks_node is not None: + for network_node in self.find_children_named(networks_node, + "network"): + item = {} + if network_node.hasAttribute("id"): + item["id"] = network_node.getAttribute("id") + if network_node.hasAttribute("fixed_ip"): + item["fixed_ip"] = network_node.getAttribute("fixed_ip") + networks.append(item) + return networks -- cgit