From 7a8ecdc03f838184b2e6eeac62d7f57ddc64967b Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Fri, 8 Jul 2011 01:39:58 -0700 Subject: start of re-work of compute/api's 'get_all' to handle more search options --- nova/compute/api.py | 83 +++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 59 insertions(+), 24 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index b0eedcd64..42ccc9f9e 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -622,34 +622,71 @@ class API(base.Base): """ return self.get(context, instance_id) - def get_all(self, context, project_id=None, reservation_id=None, - fixed_ip=None, recurse_zones=False): + def _get_all_by_reservation_id(self, context, search_opts): + search_opts['recurse_zones'] = True + return self.db.instance_get_all_by_reservation( + context, reservation_id) + + def _get_all_by_fixed_ip(self, context, search_opts): + try: + instances = self.db.fixed_ip_get_instance(context, fixed_ip) + except exception.FloatingIpNotFound, e: + instances = None + return instances + + def _get_all_by_project_id(self, context, search_opts): + return self.db.instance_get_all_by_project( + context, project_id) + + def _get_all_by_ip(self, context, search_opts): + pass + + def _get_all_by_ip6(self, context, search_opts): + pass + + def _get_all_by_name(self, context, search_opts): + pass + + def get_all(self, context, search_opts=None): """Get all instances filtered by one of the given parameters. If there is no filter and the context is an admin, it will retreive all instances in the system. """ - if reservation_id is not None: - recurse_zones = True - instances = self.db.instance_get_all_by_reservation( - context, reservation_id) - elif fixed_ip is not None: - try: - instances = self.db.fixed_ip_get_instance(context, fixed_ip) - except exception.FloatingIpNotFound, e: - if not recurse_zones: + if search_opts is None: + search_opts = {} + + exclusive_opts = ['reservation_id', + 'project_id', + 'fixed_ip', + 'ip', + 'ip6', + 'name'] + + # See if a valud search option was passed in. + # Ignore unknown search options for possible forward compatability. + # Raise an exception if more than 1 search option is specified + option = None + for k in exclusive_opts.iterkeys(): + v = search_opts.get(k, None) + if v: + if option is None: + option = k + else: raise - instances = None - elif project_id or not context.is_admin: - if not context.project: + + if option: + method_name = '_get_all_by_%s' % option + method = getattr(self, method_name, None) + instances = method(context, search_opts) + elif not context.is_admin: + if context.project: + instances = self.db.instance_get_all_by_project( + context, context.project_id) + else: instances = self.db.instance_get_all_by_user( context, context.user_id) - else: - if project_id is None: - project_id = context.project_id - instances = self.db.instance_get_all_by_project( - context, project_id) else: instances = self.db.instance_get_all(context) @@ -658,17 +695,15 @@ class API(base.Base): elif not isinstance(instances, list): instances = [instances] - if not recurse_zones: + if not search_opts.get('recurse_zones', False): return instances + # Recurse zones. Need admin context for this. admin_context = context.elevated() children = scheduler_api.call_zone_method(admin_context, "list", novaclient_collection_name="servers", - reservation_id=reservation_id, - project_id=project_id, - fixed_ip=fixed_ip, - recurse_zones=True) + **search_opts) for zone, servers in children: for server in servers: -- cgit From 04b50db56ee90c0f4dd685a8f45883522260164f Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 11 Jul 2011 14:27:01 -0700 Subject: Replace 'like' support with 'regexp' matching done in python. Since 'like' would result in a full table scan anyway, this is a bit more flexible. Make search options and matching a little more generic Return 404 when --fixed_ip doesn't match any instance, instead of a 500 only when the IP isn't in the FixedIps table. --- nova/api/ec2/cloud.py | 23 +++- nova/api/openstack/servers.py | 20 ++-- nova/compute/api.py | 75 ++++++++----- nova/db/api.py | 44 ++++++-- nova/db/sqlalchemy/api.py | 239 +++++++++++++++++++++++++++++++----------- nova/db/sqlalchemy/models.py | 1 + 6 files changed, 292 insertions(+), 110 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 9be30cf75..9efbb5985 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -118,8 +118,9 @@ class CloudController(object): def _get_mpi_data(self, context, project_id): result = {} + search_opts = {'project_id': project_id} for instance in self.compute_api.get_all(context, - project_id=project_id): + search_opts=search_opts): if instance['fixed_ips']: line = '%s slots=%d' % (instance['fixed_ips'][0]['address'], instance['vcpus']) @@ -145,7 +146,12 @@ class CloudController(object): def get_metadata(self, address): ctxt = context.get_admin_context() - instance_ref = self.compute_api.get_all(ctxt, fixed_ip=address) + search_opts = {'fixed_ip': address} + try: + instance_ref = self.compute_api.get_all(ctxt, + search_opts=search_opts) + except exception.NotFound: + instance_ref = None if instance_ref is None: return None @@ -816,11 +822,18 @@ class CloudController(object): instances = [] for ec2_id in instance_id: internal_id = ec2utils.ec2_id_to_id(ec2_id) - instance = self.compute_api.get(context, - instance_id=internal_id) + try: + instance = self.compute_api.get(context, + instance_id=internal_id) + except exception.NotFound: + continue instances.append(instance) else: - instances = self.compute_api.get_all(context, **kwargs) + try: + instances = self.compute_api.get_all(context, + search_opts=kwargs) + except exception.NotFound: + instances = [] for instance in instances: if not context.is_admin: if instance['image_ref'] == str(FLAGS.vpn_image_id): diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index fc1ab8d46..d259590a5 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -52,6 +52,8 @@ class Controller(object): servers = self._items(req, is_detail=False) except exception.Invalid as err: return exc.HTTPBadRequest(explanation=str(err)) + except exception.NotFound: + return exc.HTTPNotFound() return servers def detail(self, req): @@ -60,6 +62,8 @@ class Controller(object): servers = self._items(req, is_detail=True) except exception.Invalid as err: return exc.HTTPBadRequest(explanation=str(err)) + except exception.NotFound as err: + return exc.HTTPNotFound() return servers def _get_view_builder(self, req): @@ -77,16 +81,14 @@ class Controller(object): builder - the response model builder """ query_str = req.str_GET - reservation_id = query_str.get('reservation_id') - project_id = query_str.get('project_id') - fixed_ip = query_str.get('fixed_ip') - recurse_zones = utils.bool_from_str(query_str.get('recurse_zones')) + recurse_zones = utils.bool_from_str( + query_str.get('recurse_zones', False)) + # Pass all of the options on to compute's 'get_all' + search_opts = query_str + # Reset this after converting from string to bool + search_opts['recurse_zones'] = recurse_zones instance_list = self.compute_api.get_all( - req.environ['nova.context'], - reservation_id=reservation_id, - project_id=project_id, - fixed_ip=fixed_ip, - recurse_zones=recurse_zones) + req.environ['nova.context'], search_opts=search_opts) limited_list = self._limit_items(instance_list, req) builder = self._get_view_builder(req) servers = [builder.build(inst, is_detail)['server'] diff --git a/nova/compute/api.py b/nova/compute/api.py index 42ccc9f9e..e80e52566 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -625,27 +625,34 @@ class API(base.Base): def _get_all_by_reservation_id(self, context, search_opts): search_opts['recurse_zones'] = True return self.db.instance_get_all_by_reservation( - context, reservation_id) + context, search_opts['reservation_id']) + + def _get_all_by_project_id(self, context, search_opts): + return self.db.instance_get_all_by_project( + context, search_opts['project_id']) def _get_all_by_fixed_ip(self, context, search_opts): + fixed_ip = search_opts['fixed_ip'] try: - instances = self.db.fixed_ip_get_instance(context, fixed_ip) - except exception.FloatingIpNotFound, e: - instances = None + instances = self.db.instance_get_by_fixed_ip(context, fixed_ip) + except exception.FixedIpNotFound, e: + raise + if not instances: + raise exception.FixedIpNotFoundForAddress(address=fixed_ip) return instances - def _get_all_by_project_id(self, context, search_opts): - return self.db.instance_get_all_by_project( - context, project_id) def _get_all_by_ip(self, context, search_opts): - pass + return self.db.instance_get_all_by_ip_regexp( + context, search_opts['ip']) def _get_all_by_ip6(self, context, search_opts): - pass + return self.db.instance_get_all_by_ipv6_regexp( + context, search_opts['ip6']) - def _get_all_by_name(self, context, search_opts): - pass + def _get_all_by_column(self, context, column, search_opts): + return self.db.instance_get_all_by_column_regexp( + context, column, search_opts[column]) def get_all(self, context, search_opts=None): """Get all instances filtered by one of the given parameters. @@ -657,29 +664,45 @@ class API(base.Base): if search_opts is None: search_opts = {} + LOG.debug(_("Searching by: %s") % str(search_opts)) + + # Columns we can do a generic search on + search_columns = ['display_name', + 'server_name'] + + # Options that are mutually exclusive exclusive_opts = ['reservation_id', 'project_id', 'fixed_ip', 'ip', - 'ip6', - 'name'] + 'ip6'] + search_columns - # See if a valud search option was passed in. + # See if a valid search option was passed in. # Ignore unknown search options for possible forward compatability. # Raise an exception if more than 1 search option is specified - option = None - for k in exclusive_opts.iterkeys(): - v = search_opts.get(k, None) + found_opt = None + for opt in exclusive_opts: + v = search_opts.get(opt, None) if v: - if option is None: - option = k + if found_opt is None: + found_opt = opt else: - raise - - if option: - method_name = '_get_all_by_%s' % option - method = getattr(self, method_name, None) - instances = method(context, search_opts) + LOG.error(_("More than 1 mutually exclusive " + "search option specified (%(found_opt)s and " + "%(opt)s were both specified") % locals()) + raise exception.InvalidInput(reason= + _("More than 1 mutually exclusive " + "search option specified (%(found_opt)s and " + "%(opt)s were both specified") % locals()) + + if found_opt: + if found_opt in search_columns: + instances = self._get_all_by_column(context, + found_opt, search_opts) + else: + method_name = '_get_all_by_%s' % found_opt + method = getattr(self, method_name, None) + instances = method(context, search_opts) elif not context.is_admin: if context.project: instances = self.db.instance_get_all_by_project( @@ -703,7 +726,7 @@ class API(base.Base): children = scheduler_api.call_zone_method(admin_context, "list", novaclient_collection_name="servers", - **search_opts) + search_opts=search_opts) for zone, servers in children: for server in servers: diff --git a/nova/db/api.py b/nova/db/api.py index b7c5700e5..c0f49d98d 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -381,15 +381,6 @@ def fixed_ip_get_by_virtual_interface(context, vif_id): return IMPL.fixed_ip_get_by_virtual_interface(context, vif_id) -def fixed_ip_get_instance(context, address): - """Get an instance for a fixed ip by address.""" - return IMPL.fixed_ip_get_instance(context, address) - - -def fixed_ip_get_instance_v6(context, address): - return IMPL.fixed_ip_get_instance_v6(context, address) - - def fixed_ip_get_network(context, address): """Get a network for a fixed ip by address.""" return IMPL.fixed_ip_get_network(context, address) @@ -515,10 +506,43 @@ def instance_get_all_by_host(context, host): def instance_get_all_by_reservation(context, reservation_id): - """Get all instance belonging to a reservation.""" + """Get all instances belonging to a reservation.""" return IMPL.instance_get_all_by_reservation(context, reservation_id) +def instance_get_by_fixed_ip(context, address): + """Get an instance for a fixed ip by address.""" + return IMPL.instance_get_by_fixed_ip(context, address) + + +def instance_get_by_fixed_ipv6(context, address): + """Get an instance for a fixed ip by IPv6 address.""" + return IMPL.instance_get_by_fixed_ipv6(context, address) + + +def instance_get_all_by_column_regexp(context, column, column_regexp): + """Get all instances by using regular expression matching against + a particular DB column + """ + return IMPL.instance_get_all_by_column_regexp(context, + column, + column_regexp) + + +def instance_get_all_by_ip_regexp(context, ip_regexp): + """Get all instances by using regular expression matching against + Floating and Fixed IP Addresses + """ + return IMPL.instance_get_all_by_ip_regexp(context, ip_regexp) + + +def instance_get_all_by_ipv6_regexp(context, ipv6_regexp): + """Get all instances by using regular expression matching against + IPv6 Addresses + """ + return IMPL.instance_get_all_by_ipv6_regexp(context, ipv6_regexp) + + def instance_get_fixed_addresses(context, instance_id): """Get the fixed ip address of an instance.""" return IMPL.instance_get_fixed_addresses(context, instance_id) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index ffd009513..af2acbcb3 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -18,6 +18,7 @@ """ Implementation of SQLAlchemy backend. """ +import re import traceback import warnings @@ -797,28 +798,6 @@ def fixed_ip_get_by_virtual_interface(context, vif_id): return rv -@require_context -def fixed_ip_get_instance(context, address): - fixed_ip_ref = fixed_ip_get_by_address(context, address) - return fixed_ip_ref.instance - - -@require_context -def fixed_ip_get_instance_v6(context, address): - session = get_session() - - # convert IPv6 address to mac - mac = ipv6.to_mac(address) - - # get virtual interface - vif_ref = virtual_interface_get_by_address(context, mac) - - # look up instance based on instance_id from vif row - result = session.query(models.Instance).\ - filter_by(id=vif_ref['instance_id']) - return result - - @require_admin_context def fixed_ip_get_network(context, address): fixed_ip_ref = fixed_ip_get_by_address(context, address) @@ -1204,30 +1183,166 @@ def instance_get_all_by_project(context, project_id): @require_context def instance_get_all_by_reservation(context, reservation_id): session = get_session() + query = session.query(models.Instance).\ + filter_by(reservation_id=reservation_id).\ + options(joinedload_all('fixed_ips.floating_ips')).\ + options(joinedload('virtual_interfaces')).\ + options(joinedload('security_groups')).\ + options(joinedload_all('fixed_ips.network')).\ + options(joinedload('metadata')).\ + options(joinedload('instance_type')) if is_admin_context(context): - return session.query(models.Instance).\ - options(joinedload_all('fixed_ips.floating_ips')).\ - options(joinedload('virtual_interfaces')).\ - options(joinedload('security_groups')).\ - options(joinedload_all('fixed_ips.network')).\ - options(joinedload('metadata')).\ - options(joinedload('instance_type')).\ - filter_by(reservation_id=reservation_id).\ - filter_by(deleted=can_read_deleted(context)).\ - all() + return query.\ + filter_by(deleted=can_read_deleted(context)).\ + all() elif is_user_context(context): - return session.query(models.Instance).\ - options(joinedload_all('fixed_ips.floating_ips')).\ - options(joinedload('virtual_interfaces')).\ - options(joinedload('security_groups')).\ - options(joinedload_all('fixed_ips.network')).\ - options(joinedload('metadata')).\ - options(joinedload('instance_type')).\ - filter_by(project_id=context.project_id).\ - filter_by(reservation_id=reservation_id).\ - filter_by(deleted=False).\ - all() + return query.\ + filter_by(project_id=context.project_id).\ + filter_by(deleted=False).\ + all() + + +@require_context +def instance_get_by_fixed_ip(context, address): + fixed_ip_ref = fixed_ip_get_by_address(context, address) + return fixed_ip_ref.instance + + +@require_context +def instance_get_by_fixed_ipv6(context, address): + session = get_session() + + # convert IPv6 address to mac + mac = ipv6.to_mac(address) + + # get virtual interface + vif_ref = virtual_interface_get_by_address(context, mac) + + # look up instance based on instance_id from vif row + result = session.query(models.Instance).\ + filter_by(id=vif_ref['instance_id']) + return result + + +@require_context +def instance_get_all_by_column_regexp(context, column, column_regexp): + """Get all instances by using regular expression matching against + a particular DB column + """ + session = get_session() + + # MySQL 'regexp' is not portable, so we must do our own matching. + # First... grab all Instances. + query = session.query(models.Instance).\ + options(joinedload('metadata')) + if is_admin_context(context): + all_instances = query.\ + filter_by(deleted=can_read_deleted(context)).\ + all() + elif is_user_context(context): + all_instances = query.\ + filter_by(project_id=context.project_id).\ + filter_by(deleted=False).\ + all() + else: + return [] + + if all_instances is None: + all_instances = [] + + # Now do the regexp matching + compiled_regexp = re.compile(column_regexp) + instances = [] + + for instance in all_instances: + v = getattr(instance, column) + if v and compiled_regexp.match(v): + instances.append(instance) + return instances + + +@require_context +def instance_get_all_by_ip_regexp(context, ip_regexp): + """Get all instances by using regular expression matching against + Floating and Fixed IP Addresses + """ + session = get_session() + + fixed_ip_query = session.query(models.FixedIp).\ + options(joinedload('instance.metadata')) + floating_ip_query = session.query(models.FloatingIp).\ + options(joinedload_all('fixed_ip.instance.metadata')) + + # Query both FixedIp and FloatingIp tables to get matches. + # Since someone could theoretically search for something that matches + # instances in both tables... we need to use a dictionary keyed + # on instance ID to make sure we return only 1. We can't key off + # of 'instance' because it's just a reference and will be different + # addresses even though they might point to the same instance ID. + instances = {} + + # MySQL 'regexp' is not portable, so we must do our own matching. + # First... grab all of the IP entries. + if is_admin_context(context): + fixed_ips = fixed_ip_query.\ + filter_by(deleted=can_read_deleted(context)).\ + all() + floating_ips = floating_ip_query.\ + filter_by(deleted=can_read_deleted(context)).\ + all() + elif is_user_context(context): + fixed_ips = fixed_ip_query.filter_by(deleted=False).all() + floating_ips = floating_ip_query.filter_by(deleted=False).all() + else: + return None + + if fixed_ips is None: + fixed_ips = [] + if floating_ips is None: + floating_ips = [] + + compiled_regexp = re.compile(ip_regexp) + instances = {} + + # Now do the regexp matching + for fixed_ip in fixed_ips: + if fixed_ip.instance and compiled_regexp.match(fixed_ip.address): + instances[fixed_ip.instance.uuid] = fixed_ip.instance + for floating_ip in floating_ips: + fixed_ip = floating_ip.fixed_ip + if fixed_ip and fixed_ip.instance and\ + compiled_regexp.match(floating_ip.address): + instances[fixed_ip.instance.uuid] = fixed_ip.instance + + return instances.values() + +@require_context +def instance_get_all_by_ipv6_regex(context, ipv6_regexp): + """Get all instances by using regular expression matching against + IPv6 Addresses + """ + + session = get_session() + with session.begin(): + # get instances + + all_instances = session.query(models.Instance).\ + options(joinedload('metadata')).\ + filter_by(deleted=can_read_deleted(context)).\ + all() + if not all_instances: + return [] + + instances = [] + compiled_regexp = re.compile(ipv6_regexp) + for instance in all_instances: + ipv6_addrs = _ipv6_get_by_instance_ref(context, instance) + for ipv6 in ipv6_addrs: + if compiled_regexp.match(ipv6): + instances.append(instance) + break + return instances @require_admin_context @@ -1258,29 +1373,33 @@ def instance_get_fixed_addresses(context, instance_id): return [fixed_ip.address for fixed_ip in fixed_ips] +def _ipv6_get_by_instance_ref(context, instance_ref): + # assume instance has 1 mac for each network associated with it + # get networks associated with instance + network_refs = network_get_all_by_instance(context, instance_id) + # compile a list of cidr_v6 prefixes sorted by network id + prefixes = [ref.cidr_v6 for ref in + sorted(network_refs, key=lambda ref: ref.id)] + # get vifs associated with instance + vif_refs = virtual_interface_get_by_instance(context, instance_ref.id) + # compile list of the mac_addresses for vifs sorted by network id + macs = [vif_ref['address'] for vif_ref in + sorted(vif_refs, key=lambda vif_ref: vif_ref['network_id'])] + # get project id from instance + project_id = instance_ref.project_id + # combine prefixes, macs, and project_id into (prefix,mac,p_id) tuples + prefix_mac_tuples = zip(prefixes, macs, [project_id for m in macs]) + # return list containing ipv6 address for each tuple + return [ipv6.to_global_ipv6(*t) for t in prefix_mac_tuples] + + @require_context def instance_get_fixed_addresses_v6(context, instance_id): session = get_session() with session.begin(): # get instance instance_ref = instance_get(context, instance_id, session=session) - # assume instance has 1 mac for each network associated with it - # get networks associated with instance - network_refs = network_get_all_by_instance(context, instance_id) - # compile a list of cidr_v6 prefixes sorted by network id - prefixes = [ref.cidr_v6 for ref in - sorted(network_refs, key=lambda ref: ref.id)] - # get vifs associated with instance - vif_refs = virtual_interface_get_by_instance(context, instance_ref.id) - # compile list of the mac_addresses for vifs sorted by network id - macs = [vif_ref['address'] for vif_ref in - sorted(vif_refs, key=lambda vif_ref: vif_ref['network_id'])] - # get project id from instance - project_id = instance_ref.project_id - # combine prefixes, macs, and project_id into (prefix,mac,p_id) tuples - prefix_mac_tuples = zip(prefixes, macs, [project_id for m in macs]) - # return list containing ipv6 address for each tuple - return [ipv6.to_global_ipv6(*t) for t in prefix_mac_tuples] + return _ipv6_get_by_instance_ref(context, instance_ref) @require_context diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index d29d3d6f1..e42da193f 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -187,6 +187,7 @@ class Instance(BASE, NovaBase): image_ref = Column(String(255)) kernel_id = Column(String(255)) ramdisk_id = Column(String(255)) + server_name = Column(String(255)) # image_ref = Column(Integer, ForeignKey('images.id'), nullable=True) # kernel_id = Column(Integer, ForeignKey('images.id'), nullable=True) -- cgit From 04804aba3c995260cf376b8d979f032942cd0988 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 11 Jul 2011 14:45:27 -0700 Subject: pep8 fixes --- nova/compute/api.py | 5 ++--- nova/db/sqlalchemy/api.py | 1 + 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index e80e52566..fba56a2bb 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -641,7 +641,6 @@ class API(base.Base): raise exception.FixedIpNotFoundForAddress(address=fixed_ip) return instances - def _get_all_by_ip(self, context, search_opts): return self.db.instance_get_all_by_ip_regexp( context, search_opts['ip']) @@ -690,8 +689,8 @@ class API(base.Base): LOG.error(_("More than 1 mutually exclusive " "search option specified (%(found_opt)s and " "%(opt)s were both specified") % locals()) - raise exception.InvalidInput(reason= - _("More than 1 mutually exclusive " + raise exception.InvalidInput(reason=_( + "More than 1 mutually exclusive " "search option specified (%(found_opt)s and " "%(opt)s were both specified") % locals()) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index af2acbcb3..99e96e679 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1317,6 +1317,7 @@ def instance_get_all_by_ip_regexp(context, ip_regexp): return instances.values() + @require_context def instance_get_all_by_ipv6_regex(context, ipv6_regexp): """Get all instances by using regular expression matching against -- cgit From a3096d593fbe21625e3c4102e69d12950e9d2ef2 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Tue, 12 Jul 2011 02:01:09 -0700 Subject: added searching by instance name added unit tests --- nova/compute/api.py | 5 + nova/db/api.py | 7 ++ nova/db/sqlalchemy/api.py | 24 +++- nova/tests/test_compute.py | 265 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 300 insertions(+), 1 deletion(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index fba56a2bb..605b0d29c 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -641,6 +641,10 @@ class API(base.Base): raise exception.FixedIpNotFoundForAddress(address=fixed_ip) return instances + def _get_all_by_name(self, context, search_opts): + return self.db.instance_get_all_by_name_regexp( + context, search_opts['name']) + def _get_all_by_ip(self, context, search_opts): return self.db.instance_get_all_by_ip_regexp( context, search_opts['ip']) @@ -673,6 +677,7 @@ class API(base.Base): exclusive_opts = ['reservation_id', 'project_id', 'fixed_ip', + 'name', 'ip', 'ip6'] + search_columns diff --git a/nova/db/api.py b/nova/db/api.py index c0f49d98d..6aa44f06b 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -529,6 +529,13 @@ def instance_get_all_by_column_regexp(context, column, column_regexp): column_regexp) +def instance_get_all_by_name_regexp(context, name_regexp): + """Get all instances by using regular expression matching against + its name + """ + return IMPL.instance_get_all_by_name_regexp(context, name_regexp) + + def instance_get_all_by_ip_regexp(context, ip_regexp): """Get all instances by using regular expression matching against Floating and Fixed IP Addresses diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 99e96e679..93614a307 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1262,6 +1262,28 @@ def instance_get_all_by_column_regexp(context, column, column_regexp): return instances +@require_context +def instance_get_all_by_name_regexp(context, ipv6_regexp): + """Get all instances by using regular expression matching against + its name + """ + + session = get_session() + with session.begin(): + # get instances + + all_instances = session.query(models.Instance).\ + options(joinedload('metadata')).\ + filter_by(deleted=can_read_deleted(context)).\ + all() + if not all_instances: + return [] + + compiled_regexp = re.compile(ipv6_regexp) + return [instance for instance in all_instances + if compiled_regexp.match(instance.name)] + + @require_context def instance_get_all_by_ip_regexp(context, ip_regexp): """Get all instances by using regular expression matching against @@ -1319,7 +1341,7 @@ def instance_get_all_by_ip_regexp(context, ip_regexp): @require_context -def instance_get_all_by_ipv6_regex(context, ipv6_regexp): +def instance_get_all_by_ipv6_regexp(context, ipv6_regexp): """Get all instances by using regular expression matching against IPv6 Addresses """ diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index 45cd2f764..0190a5f73 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -30,6 +30,7 @@ from nova.compute import power_state from nova import context from nova import db from nova.db.sqlalchemy import models +from nova.db.sqlalchemy import api as sqlalchemy_api from nova import exception from nova import flags import nova.image.fake @@ -810,3 +811,267 @@ class ComputeTestCase(test.TestCase): LOG.info(_("After force-killing instances: %s"), instances) self.assertEqual(len(instances), 1) self.assertEqual(power_state.SHUTOFF, instances[0]['state']) + + def test_get_all_by_display_name_regexp(self): + """Test searching instances by display_name""" + c = context.get_admin_context() + instance_id1 = self._create_instance({'display_name': 'woot'}) + instance_id2 = self._create_instance({ + 'display_name': 'woo', + 'id': 20}) + instance_id3 = self._create_instance({ + 'display_name': 'not-woot', + 'id': 30}) + + instances = self.compute_api.get_all(c, + search_opts={'display_name': 'woo.*'}) + self.assertEqual(len(instances), 2) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id1 in instance_ids) + self.assertTrue(instance_id2 in instance_ids) + + instances = self.compute_api.get_all(c, + search_opts={'display_name': 'woot.*'}) + instance_ids = [instance.id for instance in instances] + self.assertEqual(len(instances), 1) + self.assertTrue(instance_id1 in instance_ids) + + instances = self.compute_api.get_all(c, + search_opts={'display_name': '.*oot.*'}) + self.assertEqual(len(instances), 2) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id1 in instance_ids) + self.assertTrue(instance_id3 in instance_ids) + + instances = self.compute_api.get_all(c, + search_opts={'display_name': 'n.*'}) + self.assertEqual(len(instances), 1) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id3 in instance_ids) + + instances = self.compute_api.get_all(c, + search_opts={'display_name': 'noth.*'}) + self.assertEqual(len(instances), 0) + + db.instance_destroy(c, instance_id1) + db.instance_destroy(c, instance_id2) + db.instance_destroy(c, instance_id3) + + def test_get_all_by_server_name_regexp(self): + """Test searching instances by server_name""" + c = context.get_admin_context() + instance_id1 = self._create_instance({'server_name': 'woot'}) + instance_id2 = self._create_instance({ + 'server_name': 'woo', + 'id': 20}) + instance_id3 = self._create_instance({ + 'server_name': 'not-woot', + 'id': 30}) + + instances = self.compute_api.get_all(c, + search_opts={'server_name': 'woo.*'}) + self.assertEqual(len(instances), 2) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id1 in instance_ids) + self.assertTrue(instance_id2 in instance_ids) + + instances = self.compute_api.get_all(c, + search_opts={'server_name': 'woot.*'}) + instance_ids = [instance.id for instance in instances] + self.assertEqual(len(instances), 1) + self.assertTrue(instance_id1 in instance_ids) + + instances = self.compute_api.get_all(c, + search_opts={'server_name': '.*oot.*'}) + self.assertEqual(len(instances), 2) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id1 in instance_ids) + self.assertTrue(instance_id3 in instance_ids) + + instances = self.compute_api.get_all(c, + search_opts={'server_name': 'n.*'}) + self.assertEqual(len(instances), 1) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id3 in instance_ids) + + instances = self.compute_api.get_all(c, + search_opts={'server_name': 'noth.*'}) + self.assertEqual(len(instances), 0) + + db.instance_destroy(c, instance_id1) + db.instance_destroy(c, instance_id2) + db.instance_destroy(c, instance_id3) + + def test_get_all_by_name_regexp(self): + """Test searching instances by name""" + self.flags(instance_name_template='instance-%d') + + c = context.get_admin_context() + instance_id1 = self._create_instance() + instance_id2 = self._create_instance({'id': 2}) + instance_id3 = self._create_instance({'id': 10}) + + instances = self.compute_api.get_all(c, + search_opts={'name': 'instance.*'}) + self.assertEqual(len(instances), 3) + + instances = self.compute_api.get_all(c, + search_opts={'name': '.*\-\d$'}) + self.assertEqual(len(instances), 2) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id1 in instance_ids) + self.assertTrue(instance_id2 in instance_ids) + + instances = self.compute_api.get_all(c, + search_opts={'name': 'i.*2'}) + self.assertEqual(len(instances), 1) + self.assertEqual(instances[0].id, instance_id2) + + db.instance_destroy(c, instance_id1) + db.instance_destroy(c, instance_id2) + db.instance_destroy(c, instance_id3) + + def test_get_by_fixed_ip(self): + """Test getting 1 instance by Fixed IP""" + c = context.get_admin_context() + instance_id1 = self._create_instance({'server_name': 'woot'}) + instance_id2 = self._create_instance({ + 'server_name': 'woo', + 'id': 20}) + instance_id3 = self._create_instance({ + 'server_name': 'not-woot', + 'id': 30}) + + db.fixed_ip_create(c, + {'address': '1.1.1.1', + 'instance_id': instance_id1}) + db.fixed_ip_create(c, + {'address': '1.1.2.1', + 'instance_id': instance_id2}) + + # regex not allowed + self.assertRaises(exception.NotFound, + self.compute_api.get_all, + c, + search_opts={'fixed_ip': '.*'}) + + self.assertRaises(exception.NotFound, + self.compute_api.get_all, + c, + search_opts={'fixed_ip': '1.1.3.1'}) + + instances = self.compute_api.get_all(c, + search_opts={'fixed_ip': '1.1.1.1'}) + self.assertEqual(len(instances), 1) + self.assertEqual(instances[0].id, instance_id1) + + instances = self.compute_api.get_all(c, + search_opts={'fixed_ip': '1.1.2.1'}) + self.assertEqual(len(instances), 1) + self.assertEqual(instances[0].id, instance_id2) + + db.instance_destroy(c, instance_id1) + db.instance_destroy(c, instance_id2) + + def test_get_all_by_ip_regex(self): + """Test searching by Floating and Fixed IP""" + c = context.get_admin_context() + instance_id1 = self._create_instance({'server_name': 'woot'}) + instance_id2 = self._create_instance({ + 'server_name': 'woo', + 'id': 20}) + instance_id3 = self._create_instance({ + 'server_name': 'not-woot', + 'id': 30}) + + db.fixed_ip_create(c, + {'address': '1.1.1.1', + 'instance_id': instance_id1}) + db.fixed_ip_create(c, + {'address': '1.1.2.1', + 'instance_id': instance_id2}) + fix_addr = db.fixed_ip_create(c, + {'address': '1.1.3.1', + 'instance_id': instance_id3}) + fix_ref = db.fixed_ip_get_by_address(c, fix_addr) + flo_ref = db.floating_ip_create(c, + {'address': '10.0.0.2', + 'fixed_ip_id': fix_ref['id']}) + + instances = self.compute_api.get_all(c, + search_opts={'ip': '.*\.1'}) + self.assertEqual(len(instances), 3) + + instances = self.compute_api.get_all(c, + search_opts={'ip': '1.*'}) + self.assertEqual(len(instances), 3) + + instances = self.compute_api.get_all(c, + search_opts={'ip': '.*\.1.\d+$'}) + self.assertEqual(len(instances), 1) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id1 in instance_ids) + + instances = self.compute_api.get_all(c, + search_opts={'ip': '.*\.2.+'}) + self.assertEqual(len(instances), 1) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id2 in instance_ids) + + instances = self.compute_api.get_all(c, + search_opts={'ip': '10.*'}) + self.assertEqual(len(instances), 1) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id3 in instance_ids) + + db.instance_destroy(c, instance_id1) + db.instance_destroy(c, instance_id2) + db.instance_destroy(c, instance_id3) + db.floating_ip_destroy(c, '10.0.0.2') + + def test_get_all_by_ipv6_regex(self): + """Test searching by IPv6 address""" + def fake_ipv6_get_by_instance_ref(context, instance): + if instance.id == 1: + return ['ffff:ffff::1'] + if instance.id == 20: + return ['dddd:dddd::1'] + if instance.id == 30: + return ['cccc:cccc::1', 'eeee:eeee::1', 'dddd:dddd::1'] + + self.stubs.Set(sqlalchemy_api, '_ipv6_get_by_instance_ref', + fake_ipv6_get_by_instance_ref) + + c = context.get_admin_context() + instance_id1 = self._create_instance({'server_name': 'woot'}) + instance_id2 = self._create_instance({ + 'server_name': 'woo', + 'id': 20}) + instance_id3 = self._create_instance({ + 'server_name': 'not-woot', + 'id': 30}) + + instances = self.compute_api.get_all(c, + search_opts={'ip6': 'ff.*'}) + self.assertEqual(len(instances), 1) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id1 in instance_ids) + + instances = self.compute_api.get_all(c, + search_opts={'ip6': '.*::1'}) + self.assertEqual(len(instances), 3) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id1 in instance_ids) + self.assertTrue(instance_id2 in instance_ids) + self.assertTrue(instance_id3 in instance_ids) + + instances = self.compute_api.get_all(c, + search_opts={'ip6': '.*dd:.*'}) + self.assertEqual(len(instances), 2) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id2 in instance_ids) + self.assertTrue(instance_id3 in instance_ids) + + db.instance_destroy(c, instance_id1) + db.instance_destroy(c, instance_id2) + db.instance_destroy(c, instance_id3) -- cgit From edccef06c24df2fa785005f7a3c1f52a45bfc071 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Tue, 12 Jul 2011 03:13:43 -0700 Subject: fix bugs with fixed_ip returning a 404 instance searching needs to joinload more stuff --- nova/compute/api.py | 17 +++++++- nova/db/sqlalchemy/api.py | 102 +++++++++++++++++++++++----------------------- 2 files changed, 68 insertions(+), 51 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 605b0d29c..511c17e7a 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -19,6 +19,7 @@ """Handles all requests relating to instances (guest vms).""" import eventlet +import novaclient import re import time @@ -636,7 +637,10 @@ class API(base.Base): try: instances = self.db.instance_get_by_fixed_ip(context, fixed_ip) except exception.FixedIpNotFound, e: - raise + if search_opts['recurse_zones']: + return [] + else: + raise if not instances: raise exception.FixedIpNotFoundForAddress(address=fixed_ip) return instances @@ -729,14 +733,25 @@ class API(base.Base): admin_context = context.elevated() children = scheduler_api.call_zone_method(admin_context, "list", + errors_to_ignore=[novaclient.exceptions.NotFound], novaclient_collection_name="servers", search_opts=search_opts) for zone, servers in children: + # 'servers' can be None if a 404 was returned by a zone + if servers is None: + continue for server in servers: # Results are ready to send to user. No need to scrub. server._info['_is_precooked'] = True instances.append(server._info) + + # Fixed IP returns a FixedIpNotFound when an instance is not + # found... + fixed_ip = search_opts.get('fixed_ip', None) + if fixed_ip and not instances: + raise exception.FixedIpNotFoundForAddress(address=fixed_ip) + return instances def _cast_compute_message(self, method, context, instance_id, host=None, diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 93614a307..59db56a5c 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1234,27 +1234,20 @@ def instance_get_all_by_column_regexp(context, column, column_regexp): # MySQL 'regexp' is not portable, so we must do our own matching. # First... grab all Instances. - query = session.query(models.Instance).\ - options(joinedload('metadata')) - if is_admin_context(context): - all_instances = query.\ - filter_by(deleted=can_read_deleted(context)).\ - all() - elif is_user_context(context): - all_instances = query.\ - filter_by(project_id=context.project_id).\ - filter_by(deleted=False).\ - all() - else: + all_instances = session.query(models.Instance).\ + options(joinedload_all('fixed_ips.floating_ips')).\ + options(joinedload('virtual_interfaces')).\ + options(joinedload('security_groups')).\ + options(joinedload_all('fixed_ips.network')).\ + options(joinedload('metadata')).\ + options(joinedload('instance_type')).\ + filter_by(deleted=can_read_deleted(context)).\ + all() + if not all_instances: return [] - - if all_instances is None: - all_instances = [] - # Now do the regexp matching compiled_regexp = re.compile(column_regexp) instances = [] - for instance in all_instances: v = getattr(instance, column) if v and compiled_regexp.match(v): @@ -1269,19 +1262,24 @@ def instance_get_all_by_name_regexp(context, ipv6_regexp): """ session = get_session() - with session.begin(): - # get instances - - all_instances = session.query(models.Instance).\ - options(joinedload('metadata')).\ - filter_by(deleted=can_read_deleted(context)).\ - all() - if not all_instances: - return [] - compiled_regexp = re.compile(ipv6_regexp) - return [instance for instance in all_instances - if compiled_regexp.match(instance.name)] + # MySQL 'regexp' is not portable, so we must do our own matching. + # First... grab all Instances. + all_instances = session.query(models.Instance).\ + options(joinedload_all('fixed_ips.floating_ips')).\ + options(joinedload('virtual_interfaces')).\ + options(joinedload('security_groups')).\ + options(joinedload_all('fixed_ips.network')).\ + options(joinedload('metadata')).\ + options(joinedload('instance_type')).\ + filter_by(deleted=can_read_deleted(context)).\ + all() + if not all_instances: + return [] + # Now do the regexp matching + compiled_regexp = re.compile(ipv6_regexp) + return [instance for instance in all_instances + if compiled_regexp.match(instance.name)] @require_context @@ -1291,11 +1289,6 @@ def instance_get_all_by_ip_regexp(context, ip_regexp): """ session = get_session() - fixed_ip_query = session.query(models.FixedIp).\ - options(joinedload('instance.metadata')) - floating_ip_query = session.query(models.FloatingIp).\ - options(joinedload_all('fixed_ip.instance.metadata')) - # Query both FixedIp and FloatingIp tables to get matches. # Since someone could theoretically search for something that matches # instances in both tables... we need to use a dictionary keyed @@ -1304,20 +1297,26 @@ def instance_get_all_by_ip_regexp(context, ip_regexp): # addresses even though they might point to the same instance ID. instances = {} - # MySQL 'regexp' is not portable, so we must do our own matching. - # First... grab all of the IP entries. - if is_admin_context(context): - fixed_ips = fixed_ip_query.\ - filter_by(deleted=can_read_deleted(context)).\ - all() - floating_ips = floating_ip_query.\ - filter_by(deleted=can_read_deleted(context)).\ - all() - elif is_user_context(context): - fixed_ips = fixed_ip_query.filter_by(deleted=False).all() - floating_ips = floating_ip_query.filter_by(deleted=False).all() - else: - return None + fixed_ips = session.query(models.FixedIp).\ + options(joinedload_all('instance.fixed_ips.floating_ips')).\ + options(joinedload('instance.virtual_interfaces')).\ + options(joinedload('instance.security_groups')).\ + options(joinedload_all('instance.fixed_ips.network')).\ + options(joinedload('instance.metadata')).\ + options(joinedload('instance.instance_type')).\ + filter_by(deleted=can_read_deleted(context)).\ + all() + floating_ips = session.query(models.FloatingIp).\ + options(joinedload_all( + 'fixed_ip.instance.fixed_ips.floating_ips')).\ + options(joinedload('fixed_ip.instance.virtual_interfaces')).\ + options(joinedload('fixed_ip.instance.security_groups')).\ + options(joinedload_all( + 'fixed_ip.instance.fixed_ips.network')).\ + options(joinedload('fixed_ip.instance.metadata')).\ + options(joinedload('fixed_ip.instance.instance_type')).\ + filter_by(deleted=can_read_deleted(context)).\ + all() if fixed_ips is None: fixed_ips = [] @@ -1348,10 +1347,13 @@ def instance_get_all_by_ipv6_regexp(context, ipv6_regexp): session = get_session() with session.begin(): - # get instances - all_instances = session.query(models.Instance).\ + options(joinedload_all('fixed_ips.floating_ips')).\ + options(joinedload('virtual_interfaces')).\ + options(joinedload('security_groups')).\ + options(joinedload_all('fixed_ips.network')).\ options(joinedload('metadata')).\ + options(joinedload('instance_type')).\ filter_by(deleted=can_read_deleted(context)).\ all() if not all_instances: -- cgit From bbd8f482b916168871d1d83192b354355858e77c Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Tue, 12 Jul 2011 15:16:16 -0700 Subject: python-novaclient 2.5.8 is required --- tools/pip-requires | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/pip-requires b/tools/pip-requires index dec93c351..db9e950a8 100644 --- a/tools/pip-requires +++ b/tools/pip-requires @@ -9,7 +9,7 @@ boto==1.9b carrot==0.10.5 eventlet lockfile==0.8 -python-novaclient==2.5.7 +python-novaclient==2.5.8 python-daemon==1.5.5 python-gflags==1.3 redis==2.0.0 -- cgit From 1dec3d7c3380d83398be0588b58c1cad13252807 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 14 Jul 2011 12:13:13 -0700 Subject: clean up checking for exclusive search options fix a cut n paste error with instance_get_all_by_name_regexp --- nova/compute/api.py | 32 +++++++++++++++----------------- nova/db/sqlalchemy/api.py | 4 ++-- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index f795e345a..a445ef6eb 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -689,21 +689,19 @@ class API(base.Base): # Ignore unknown search options for possible forward compatability. # Raise an exception if more than 1 search option is specified found_opt = None - for opt in exclusive_opts: - v = search_opts.get(opt, None) - if v: - if found_opt is None: - found_opt = opt - else: - LOG.error(_("More than 1 mutually exclusive " - "search option specified (%(found_opt)s and " - "%(opt)s were both specified") % locals()) - raise exception.InvalidInput(reason=_( - "More than 1 mutually exclusive " - "search option specified (%(found_opt)s and " - "%(opt)s were both specified") % locals()) - - if found_opt: + found_opts = [opt for opt in exclusive_opts + if search_opts.get(opt, None)] + if len(found_opts) > 1: + found_opt_str = ", ".join(found_opts) + msg = _("More than 1 mutually exclusive " + "search option specified: %(found_opt_str)s") \ + % locals() + logger.error(msg) + raise exception.InvalidInput(reason=msg) + + # Found a search option? + if found_opts: + found_opt = found_opts[0] if found_opt in search_columns: instances = self._get_all_by_column(context, found_opt, search_opts) @@ -714,10 +712,10 @@ class API(base.Base): elif not context.is_admin: if context.project: instances = self.db.instance_get_all_by_project( - context, context.project_id) + context, context.project_id) else: instances = self.db.instance_get_all_by_user( - context, context.user_id) + context, context.user_id) else: instances = self.db.instance_get_all(context) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 59db56a5c..735d63be9 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1256,7 +1256,7 @@ def instance_get_all_by_column_regexp(context, column, column_regexp): @require_context -def instance_get_all_by_name_regexp(context, ipv6_regexp): +def instance_get_all_by_name_regexp(context, name_regexp): """Get all instances by using regular expression matching against its name """ @@ -1277,7 +1277,7 @@ def instance_get_all_by_name_regexp(context, ipv6_regexp): if not all_instances: return [] # Now do the regexp matching - compiled_regexp = re.compile(ipv6_regexp) + compiled_regexp = re.compile(name_regexp) return [instance for instance in all_instances if compiled_regexp.match(instance.name)] -- cgit From d2265cbe65f1b3940b37966245da13b9714234ef Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Sun, 17 Jul 2011 16:12:59 -0700 Subject: Refactored OS API code to allow checking of invalid query string paremeters and admin api/context to the index/detail calls. v1.0 still ignores unknown parameters, but v1.1 will return 400/BadRequest on unknown options. admin_api only commands are treated as unknown parameters if FLAGS.enable_admin_api is False. If enable_admin_api is True, non-admin context requests return 403/Forbidden. Fixed EC2 API code to handle search options to compute_api.get_all() more correctly. Reverted compute_api.get_all to ignore unknown options, since the OS API now does the verification. Updated tests. --- nova/api/ec2/cloud.py | 23 ++++--- nova/api/openstack/servers.py | 123 +++++++++++++++++++++++++++++---- nova/compute/api.py | 154 ++++++++++++++++++++++++------------------ nova/tests/test_compute.py | 8 +-- 4 files changed, 215 insertions(+), 93 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 0d24f0938..76725370a 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -800,11 +800,16 @@ class CloudController(object): return [{label: x} for x in lst] def describe_instances(self, context, **kwargs): - return self._format_describe_instances(context, **kwargs) + # Optional DescribeInstances argument + instance_id = kwargs.get('instance_id', None) + return self._format_describe_instances(context, + instance_id=instance_id) def describe_instances_v6(self, context, **kwargs): - kwargs['use_v6'] = True - return self._format_describe_instances(context, **kwargs) + # Optional DescribeInstancesV6 argument + instance_id = kwargs.get('instance_id', None) + return self._format_describe_instances(context, + instance_id=instance_id, use_v6=True) def _format_describe_instances(self, context, **kwargs): return {'reservationSet': self._format_instances(context, **kwargs)} @@ -814,7 +819,8 @@ class CloudController(object): assert len(i) == 1 return i[0] - def _format_instances(self, context, instance_id=None, **kwargs): + def _format_instances(self, context, instance_id=None, use_v6=False, + **search_opts): # TODO(termie): this method is poorly named as its name does not imply # that it will be making a variety of database calls # rather than simply formatting a bunch of instances that @@ -827,14 +833,15 @@ class CloudController(object): internal_id = ec2utils.ec2_id_to_id(ec2_id) try: instance = self.compute_api.get(context, - instance_id=internal_id) + instance_id=internal_id, + search_opts=search_opts) except exception.NotFound: continue instances.append(instance) else: try: instances = self.compute_api.get_all(context, - search_opts=kwargs) + search_opts=search_opts) except exception.NotFound: instances = [] for instance in instances: @@ -856,7 +863,7 @@ class CloudController(object): fixed_addr = fixed['address'] if fixed['floating_ips']: floating_addr = fixed['floating_ips'][0]['address'] - if fixed['network'] and 'use_v6' in kwargs: + if fixed['network'] and use_v6: i['dnsNameV6'] = ipv6.to_global( fixed['network']['cidr_v6'], fixed['virtual_interface']['address'], @@ -1014,7 +1021,7 @@ class CloudController(object): 'AvailabilityZone'), block_device_mapping=kwargs.get('block_device_mapping', {})) return self._format_run_instances(context, - instances[0]['reservation_id']) + instance_id=instances[0]['reservation_id']) def _do_instance(self, action, context, ec2_id): instance_id = ec2utils.ec2_id_to_id(ec2_id) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 8a947c0e0..218037d14 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -39,6 +39,41 @@ LOG = logging.getLogger('nova.api.openstack.servers') FLAGS = flags.FLAGS +def check_option_permissions(context, specified_options, + user_api_options, admin_api_options): + """Check whether or not entries in 'specified_options' are valid + based on the allowed 'user_api_options' and 'admin_api_options'. + + All inputs are lists of option names + + Returns: exception.InvalidInput for an invalid option or + exception.AdminRequired for needing admin privs + """ + + # We pretend we don't know about admin_api_options if the admin + # API is not enabled. + if FLAGS.enable_admin_api: + known_options = user_api_options + admin_api_options + else: + known_options = user_api_options + + # Check for unknown query string params. + spec_unknown_opts = [for opt in specified_options + if opt not in known_options] + if spec_unknown_opts: + unknown_opt_str = ", ".join(spec_unknown_opts) + raise exception.InvalidInput(reason=_( + "Unknown options specified: %(unknown_opt_str)")) + + # Check for admin context for the admin commands + if not context.is_admin: + spec_admin_opts = [for opt in specified_options + if opt in admin_api_options] + if spec_admin_opts: + admin_opt_str = ", ".join(admin_opts) + raise exception.AdminRequired() + + class Controller(object): """ The Server API controller for the OpenStack API """ @@ -51,9 +86,9 @@ class Controller(object): try: servers = self._items(req, is_detail=False) except exception.Invalid as err: - return exc.HTTPBadRequest(explanation=str(err)) + return faults.Fault(exc.HTTPBadRequest(explanation=str(err))) except exception.NotFound: - return exc.HTTPNotFound() + return faults.Fault(exc.HTTPNotFound()) return servers def detail(self, req): @@ -61,9 +96,9 @@ class Controller(object): try: servers = self._items(req, is_detail=True) except exception.Invalid as err: - return exc.HTTPBadRequest(explanation=str(err)) + return faults.Fault(exc.HTTPBadRequest(explanation=str(err))) except exception.NotFound as err: - return exc.HTTPNotFound() + return faults.Fault(exc.HTTPNotFound()) return servers def _get_view_builder(self, req): @@ -75,20 +110,17 @@ class Controller(object): def _action_rebuild(self, info, request, instance_id): raise NotImplementedError() - def _items(self, req, is_detail): - """Returns a list of servers for a given user. + def _get_items(self, context, req, is_detail, search_opts=None): + """Returns a list of servers. builder - the response model builder """ - query_str = req.str_GET - recurse_zones = utils.bool_from_str( - query_str.get('recurse_zones', False)) - # Pass all of the options on to compute's 'get_all' - search_opts = query_str - # Reset this after converting from string to bool - search_opts['recurse_zones'] = recurse_zones + + if search_opts is None: + search_opts = {} + instance_list = self.compute_api.get_all( - req.environ['nova.context'], search_opts=search_opts) + context, search_opts=search_opts) limited_list = self._limit_items(instance_list, req) builder = self._get_view_builder(req) servers = [builder.build(inst, is_detail)['server'] @@ -422,6 +454,41 @@ class ControllerV10(Controller): return faults.Fault(exc.HTTPNotFound()) return exc.HTTPAccepted() + def _items(self, req, is_detail): + """Returns a list of servers based on the request. + + Checks for search options and permissions on the options. + """ + + search_opts = {} + search_opts.update(req.str_GET) + + user_api = ['project_id', 'fixed_ip', 'recurse_zones', + 'reservation_id', 'name', 'fresh', 'ip', 'ip6'] + admin_api = ['instance_name'] + + context = req.environ['nova.context'] + + try: + check_option_permissions(context, search_opt.keys(), + user_api, admin_api) + except exception.InvalidInput: + # FIXME(comstud): I refactored code in here to support + # new search options, and the original code ignored + # invalid options. So, I've left it this way for now. + # The v1.1 implementation will return an error in this + # case.. + pass + except exception.AdminRequired, e: + raise faults.Fault(exc.HTTPForbidden(detail=str(e))) + + # Convert recurse_zones into a boolean + search_opts['recurse_zones'] = utils.bool_from_str( + search_opts.get('recurse_zones', False)) + + return self._get_items(context, req, is_detail, + search_opts=search_opts) + def _image_ref_from_req_data(self, data): return data['server']['imageId'] @@ -493,6 +560,34 @@ class ControllerV11(Controller): except exception.NotFound: return faults.Fault(exc.HTTPNotFound()) + def _items(self, req, is_detail): + """Returns a list of servers based on the request. + + Checks for search options and permissions on the options. + """ + + search_opts = {} + search_opts.update(req.str_GET) + + user_api = ['image', 'flavor', 'name', 'status', + 'reservation_id', 'changes-since', 'ip', 'ip6'] + admin_api = ['ip', 'ip6', 'instance_name'] + + context = req.environ['nova.context'] + + try: + check_option_permissions(context, search_opt.keys(), + user_api, admin_api) + except exception.InvalidInput, e: + raise faults.Fault(exc.HTTPBadRequest(detail=str(e))) + except exception.AdminRequired, e: + raise faults.Fault(exc.HTTPForbidden(detail=str(e))) + + # NOTE(comstud): Making recurse_zones always be True in v1.1 + search_opts['recurse_zones'] = True + return self._get_items(context, req, is_detail, + search_opts=search_opts) + def _image_ref_from_req_data(self, data): return data['server']['imageRef'] diff --git a/nova/compute/api.py b/nova/compute/api.py index a445ef6eb..0c76f4ad6 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -623,44 +623,6 @@ class API(base.Base): """ return self.get(context, instance_id) - def _get_all_by_reservation_id(self, context, search_opts): - search_opts['recurse_zones'] = True - return self.db.instance_get_all_by_reservation( - context, search_opts['reservation_id']) - - def _get_all_by_project_id(self, context, search_opts): - return self.db.instance_get_all_by_project( - context, search_opts['project_id']) - - def _get_all_by_fixed_ip(self, context, search_opts): - fixed_ip = search_opts['fixed_ip'] - try: - instances = self.db.instance_get_by_fixed_ip(context, fixed_ip) - except exception.FixedIpNotFound, e: - if search_opts['recurse_zones']: - return [] - else: - raise - if not instances: - raise exception.FixedIpNotFoundForAddress(address=fixed_ip) - return instances - - def _get_all_by_name(self, context, search_opts): - return self.db.instance_get_all_by_name_regexp( - context, search_opts['name']) - - def _get_all_by_ip(self, context, search_opts): - return self.db.instance_get_all_by_ip_regexp( - context, search_opts['ip']) - - def _get_all_by_ip6(self, context, search_opts): - return self.db.instance_get_all_by_ipv6_regexp( - context, search_opts['ip6']) - - def _get_all_by_column(self, context, column, search_opts): - return self.db.instance_get_all_by_column_regexp( - context, column, search_opts[column]) - def get_all(self, context, search_opts=None): """Get all instances filtered by one of the given parameters. @@ -668,57 +630,115 @@ class API(base.Base): all instances in the system. """ + def _get_all_by_reservation_id(reservation_id): + """Get instances by reservation ID""" + # reservation_id implies recurse_zones + search_opts['recurse_zones'] = True + return self.db.instance_get_all_by_reservation(context, + reservation_id) + + def _get_all_by_project_id(project_id): + """Get instances by project ID""" + return self.db.instance_get_all_by_project(context, project_id) + + def _get_all_by_fixed_ip(fixed_ip): + """Get instance by fixed IP""" + try: + instances = self.db.instance_get_by_fixed_ip(context, + fixed_ip) + except exception.FixedIpNotFound, e: + if search_opts.get('recurse_zones', False): + return [] + else: + raise + if not instances: + raise exception.FixedIpNotFoundForAddress(address=fixed_ip) + return instances + + def _get_all_by_instance_name(instance_name_regexp): + """Get instances by matching the Instance.name property""" + return self.db.instance_get_all_by_name_regexp( + context, instance_name_regexp) + + def _get_all_by_ip(ip_regexp): + """Get instances by matching IPv4 addresses""" + return self.db.instance_get_all_by_ip_regexp(context, ip_regexp) + + def _get_all_by_ipv6(ipv6_regexp): + """Get instances by matching IPv6 addresses""" + return self.db.instance_get_all_by_ipv6_regexp(context, + ipv6_regexp) + + def _get_all_by_column(column_regexp, column): + """Get instances by matching Instance.""" + return self.db.instance_get_all_by_column_regexp( + context, column, column_regexp) + + # Define the search params that we will allow. This is a mapping + # of the search param to tuple of (function_to_call, (function_args)) + # A 'None' function means it's an optional parameter that will + # influence the search results, but itself is not a search option. + # Search options are mutually exclusive + known_params = { + 'recurse_zones': (None, None), + # v1.0 API? + 'fresh': (None, None), + # v1.1 API + 'changes-since': (None, None), + # Mutually exclusive options + 'display_name': (_get_all_by_column, ('display_name',)), + 'reservation_id': (_get_all_by_reservation_id, ()), + # Needed for EC2 API + 'fixed_ip': (_get_all_by_fixed_ip, ()), + # Needed for EC2 API + 'project_id': (_get_all_by_project_id, ()), + 'ip': (_get_all_by_ip, ()), + 'ip6': (_get_all_by_ipv6, ()), + 'instance_name': (_get_all_by_instance_name, ()), + 'server_name': (_get_all_by_column, ('server_name',))} + + # FIXME(comstud): 'fresh' and 'changes-since' are currently not + # implemented... + if search_opts is None: search_opts = {} LOG.debug(_("Searching by: %s") % str(search_opts)) - # Columns we can do a generic search on - search_columns = ['display_name', - 'server_name'] - - # Options that are mutually exclusive - exclusive_opts = ['reservation_id', - 'project_id', - 'fixed_ip', - 'name', - 'ip', - 'ip6'] + search_columns - - # See if a valid search option was passed in. - # Ignore unknown search options for possible forward compatability. - # Raise an exception if more than 1 search option is specified - found_opt = None - found_opts = [opt for opt in exclusive_opts - if search_opts.get(opt, None)] + # Mutually exclusive serach options are any options that have + # a function to call. Raise an exception if more than 1 is + # specified... + # NOTE(comstud): Ignore unknown options. The OS API will + # do it's own verification on options.. + found_opts = [opt for opt in search_opts.iterkeys() + if opt in known_params and \ + known_params[opt][0] is not None] if len(found_opts) > 1: found_opt_str = ", ".join(found_opts) msg = _("More than 1 mutually exclusive " "search option specified: %(found_opt_str)s") \ % locals() - logger.error(msg) + LOG.error(msg) raise exception.InvalidInput(reason=msg) # Found a search option? if found_opts: found_opt = found_opts[0] - if found_opt in search_columns: - instances = self._get_all_by_column(context, - found_opt, search_opts) - else: - method_name = '_get_all_by_%s' % found_opt - method = getattr(self, method_name, None) - instances = method(context, search_opts) - elif not context.is_admin: + f, f_args = known_params[found_opt] + instances = f(search_opts[found_opt], *f_args) + # Nope. Return all instances if the request is in admin context.. + elif context.is_admin: + instances = self.db.instance_get_all(context) + # Nope. Return all instances for the user/project + else: if context.project: instances = self.db.instance_get_all_by_project( context, context.project_id) else: instances = self.db.instance_get_all_by_user( context, context.user_id) - else: - instances = self.db.instance_get_all(context) + # Convert any responses into a list of instances if instances is None: instances = [] elif not isinstance(instances, list): diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index bdf2edd50..fc075b6c7 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -910,7 +910,7 @@ class ComputeTestCase(test.TestCase): db.instance_destroy(c, instance_id2) db.instance_destroy(c, instance_id3) - def test_get_all_by_name_regexp(self): + def test_get_all_by_instance_name_regexp(self): """Test searching instances by name""" self.flags(instance_name_template='instance-%d') @@ -920,18 +920,18 @@ class ComputeTestCase(test.TestCase): instance_id3 = self._create_instance({'id': 10}) instances = self.compute_api.get_all(c, - search_opts={'name': 'instance.*'}) + search_opts={'instance_name': 'instance.*'}) self.assertEqual(len(instances), 3) instances = self.compute_api.get_all(c, - search_opts={'name': '.*\-\d$'}) + search_opts={'instance_name': '.*\-\d$'}) self.assertEqual(len(instances), 2) instance_ids = [instance.id for instance in instances] self.assertTrue(instance_id1 in instance_ids) self.assertTrue(instance_id2 in instance_ids) instances = self.compute_api.get_all(c, - search_opts={'name': 'i.*2'}) + search_opts={'instance_name': 'i.*2'}) self.assertEqual(len(instances), 1) self.assertEqual(instances[0].id, instance_id2) -- cgit From 491c90924ac87e533ce61e3bf949a50bfdd6a31d Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Sun, 17 Jul 2011 16:35:11 -0700 Subject: compute's get_all should accept 'name' not 'display_name' for searching Instance.display_name. Removed 'server_name' searching.. Fixed DB calls for searching to filter results based on context --- nova/compute/api.py | 5 ++-- nova/db/sqlalchemy/api.py | 58 ++++++++++++++++++++++++++++++++------ nova/tests/test_compute.py | 69 +++++++--------------------------------------- 3 files changed, 61 insertions(+), 71 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 0c76f4ad6..6661775a5 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -686,7 +686,7 @@ class API(base.Base): # v1.1 API 'changes-since': (None, None), # Mutually exclusive options - 'display_name': (_get_all_by_column, ('display_name',)), + 'name': (_get_all_by_column, ('display_name',)), 'reservation_id': (_get_all_by_reservation_id, ()), # Needed for EC2 API 'fixed_ip': (_get_all_by_fixed_ip, ()), @@ -694,8 +694,7 @@ class API(base.Base): 'project_id': (_get_all_by_project_id, ()), 'ip': (_get_all_by_ip, ()), 'ip6': (_get_all_by_ipv6, ()), - 'instance_name': (_get_all_by_instance_name, ()), - 'server_name': (_get_all_by_column, ('server_name',))} + 'instance_name': (_get_all_by_instance_name, ())} # FIXME(comstud): 'fresh' and 'changes-since' are currently not # implemented... diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 735d63be9..feccba389 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1234,15 +1234,25 @@ def instance_get_all_by_column_regexp(context, column, column_regexp): # MySQL 'regexp' is not portable, so we must do our own matching. # First... grab all Instances. - all_instances = session.query(models.Instance).\ + prefix = session.query(models.Instance).\ options(joinedload_all('fixed_ips.floating_ips')).\ options(joinedload('virtual_interfaces')).\ options(joinedload('security_groups')).\ options(joinedload_all('fixed_ips.network')).\ options(joinedload('metadata')).\ options(joinedload('instance_type')).\ - filter_by(deleted=can_read_deleted(context)).\ - all() + filter_by(deleted=can_read_deleted(context)) + + if context.is_admin: + all_instances = prefix.all() + elif context.project: + all_instances = prefix.\ + filter_by(project_id=context.project_id).\ + all() + else: + all_instances = prefix.\ + filter_by(user_id=context.user_id).\ + all() if not all_instances: return [] # Now do the regexp matching @@ -1265,15 +1275,25 @@ def instance_get_all_by_name_regexp(context, name_regexp): # MySQL 'regexp' is not portable, so we must do our own matching. # First... grab all Instances. - all_instances = session.query(models.Instance).\ + prefix = session.query(models.Instance).\ options(joinedload_all('fixed_ips.floating_ips')).\ options(joinedload('virtual_interfaces')).\ options(joinedload('security_groups')).\ options(joinedload_all('fixed_ips.network')).\ options(joinedload('metadata')).\ options(joinedload('instance_type')).\ - filter_by(deleted=can_read_deleted(context)).\ - all() + filter_by(deleted=can_read_deleted(context)) + + if context.is_admin: + all_instances = prefix.all() + elif context.project: + all_instances = prefix.\ + filter_by(project_id=context.project_id).\ + all() + else: + all_instances = prefix.\ + filter_by(user_id=context.user_id).\ + all() if not all_instances: return [] # Now do the regexp matching @@ -1336,6 +1356,15 @@ def instance_get_all_by_ip_regexp(context, ip_regexp): compiled_regexp.match(floating_ip.address): instances[fixed_ip.instance.uuid] = fixed_ip.instance + if context.is_admin: + return instances.values() + elif context.project: + return [instance for instance in instances.values() + if instance.project_id == context.project_id] + else: + return [instance for instance in instances.values() + if instance.user_id == context.user_id] + return instances.values() @@ -1347,15 +1376,26 @@ def instance_get_all_by_ipv6_regexp(context, ipv6_regexp): session = get_session() with session.begin(): - all_instances = session.query(models.Instance).\ + prefix = session.query(models.Instance).\ options(joinedload_all('fixed_ips.floating_ips')).\ options(joinedload('virtual_interfaces')).\ options(joinedload('security_groups')).\ options(joinedload_all('fixed_ips.network')).\ options(joinedload('metadata')).\ options(joinedload('instance_type')).\ - filter_by(deleted=can_read_deleted(context)).\ - all() + filter_by(deleted=can_read_deleted(context)) + + if context.is_admin: + all_instances = prefix.all() + elif context.project: + all_instances = prefix.\ + filter_by(project_id=context.project_id).\ + all() + else: + all_instances = prefix.\ + filter_by(user_id=context.user_id).\ + all() + if not all_instances: return [] diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index fc075b6c7..152687083 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -820,8 +820,8 @@ class ComputeTestCase(test.TestCase): self.assertEqual(len(instances), 1) self.assertEqual(power_state.SHUTOFF, instances[0]['state']) - def test_get_all_by_display_name_regexp(self): - """Test searching instances by display_name""" + def test_get_all_by_name_regexp(self): + """Test searching instances by name (display_name)""" c = context.get_admin_context() instance_id1 = self._create_instance({'display_name': 'woot'}) instance_id2 = self._create_instance({ @@ -832,78 +832,33 @@ class ComputeTestCase(test.TestCase): 'id': 30}) instances = self.compute_api.get_all(c, - search_opts={'display_name': 'woo.*'}) + search_opts={'name': 'woo.*'}) self.assertEqual(len(instances), 2) instance_ids = [instance.id for instance in instances] self.assertTrue(instance_id1 in instance_ids) self.assertTrue(instance_id2 in instance_ids) instances = self.compute_api.get_all(c, - search_opts={'display_name': 'woot.*'}) + search_opts={'name': 'woot.*'}) instance_ids = [instance.id for instance in instances] self.assertEqual(len(instances), 1) self.assertTrue(instance_id1 in instance_ids) instances = self.compute_api.get_all(c, - search_opts={'display_name': '.*oot.*'}) + search_opts={'name': '.*oot.*'}) self.assertEqual(len(instances), 2) instance_ids = [instance.id for instance in instances] self.assertTrue(instance_id1 in instance_ids) self.assertTrue(instance_id3 in instance_ids) instances = self.compute_api.get_all(c, - search_opts={'display_name': 'n.*'}) + search_opts={'name': 'n.*'}) self.assertEqual(len(instances), 1) instance_ids = [instance.id for instance in instances] self.assertTrue(instance_id3 in instance_ids) instances = self.compute_api.get_all(c, - search_opts={'display_name': 'noth.*'}) - self.assertEqual(len(instances), 0) - - db.instance_destroy(c, instance_id1) - db.instance_destroy(c, instance_id2) - db.instance_destroy(c, instance_id3) - - def test_get_all_by_server_name_regexp(self): - """Test searching instances by server_name""" - c = context.get_admin_context() - instance_id1 = self._create_instance({'server_name': 'woot'}) - instance_id2 = self._create_instance({ - 'server_name': 'woo', - 'id': 20}) - instance_id3 = self._create_instance({ - 'server_name': 'not-woot', - 'id': 30}) - - instances = self.compute_api.get_all(c, - search_opts={'server_name': 'woo.*'}) - self.assertEqual(len(instances), 2) - instance_ids = [instance.id for instance in instances] - self.assertTrue(instance_id1 in instance_ids) - self.assertTrue(instance_id2 in instance_ids) - - instances = self.compute_api.get_all(c, - search_opts={'server_name': 'woot.*'}) - instance_ids = [instance.id for instance in instances] - self.assertEqual(len(instances), 1) - self.assertTrue(instance_id1 in instance_ids) - - instances = self.compute_api.get_all(c, - search_opts={'server_name': '.*oot.*'}) - self.assertEqual(len(instances), 2) - instance_ids = [instance.id for instance in instances] - self.assertTrue(instance_id1 in instance_ids) - self.assertTrue(instance_id3 in instance_ids) - - instances = self.compute_api.get_all(c, - search_opts={'server_name': 'n.*'}) - self.assertEqual(len(instances), 1) - instance_ids = [instance.id for instance in instances] - self.assertTrue(instance_id3 in instance_ids) - - instances = self.compute_api.get_all(c, - search_opts={'server_name': 'noth.*'}) + search_opts={'name': 'noth.*'}) self.assertEqual(len(instances), 0) db.instance_destroy(c, instance_id1) @@ -942,13 +897,9 @@ class ComputeTestCase(test.TestCase): def test_get_by_fixed_ip(self): """Test getting 1 instance by Fixed IP""" c = context.get_admin_context() - instance_id1 = self._create_instance({'server_name': 'woot'}) - instance_id2 = self._create_instance({ - 'server_name': 'woo', - 'id': 20}) - instance_id3 = self._create_instance({ - 'server_name': 'not-woot', - 'id': 30}) + instance_id1 = self._create_instance() + instance_id2 = self._create_instance({'id': 20}) + instance_id3 = self._create_instance({'id': 30}) db.fixed_ip_create(c, {'address': '1.1.1.1', -- cgit From 102a0e5b9d6ce22a5fc5a00fc260bbe1e3592222 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 02:45:10 -0700 Subject: added searching by 'image', 'flavor', and 'status' reverted ip/ip6 searching to be admin only --- nova/api/openstack/servers.py | 18 +++-- nova/api/openstack/views/servers.py | 15 +--- nova/compute/api.py | 47 +++++++++-- nova/compute/power_state.py | 29 +++++++ nova/db/api.py | 5 ++ nova/db/sqlalchemy/api.py | 42 ++++++++++ nova/tests/test_compute.py | 156 ++++++++++++++++++++++++++++++++---- 7 files changed, 271 insertions(+), 41 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 218037d14..fb1ce2529 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -119,6 +119,14 @@ class Controller(object): if search_opts is None: search_opts = {} + # If search by 'status', we need to convert it to 'state' + # If the status is unknown, bail + status = search_opts.pop('status', None) + if status is not None: + search_opts['state'] = power_state.states_from_status(status) + if len(search_opts['state']) == 0: + raise exception.InvalidInput(reason=_( + 'Invalid server status')) instance_list = self.compute_api.get_all( context, search_opts=search_opts) limited_list = self._limit_items(instance_list, req) @@ -464,8 +472,8 @@ class ControllerV10(Controller): search_opts.update(req.str_GET) user_api = ['project_id', 'fixed_ip', 'recurse_zones', - 'reservation_id', 'name', 'fresh', 'ip', 'ip6'] - admin_api = ['instance_name'] + 'reservation_id', 'name', 'fresh', 'status'] + admin_api = ['ip', 'ip6', 'instance_name'] context = req.environ['nova.context'] @@ -570,7 +578,7 @@ class ControllerV11(Controller): search_opts.update(req.str_GET) user_api = ['image', 'flavor', 'name', 'status', - 'reservation_id', 'changes-since', 'ip', 'ip6'] + 'reservation_id', 'changes-since'] admin_api = ['ip', 'ip6', 'instance_name'] context = req.environ['nova.context'] @@ -579,9 +587,9 @@ class ControllerV11(Controller): check_option_permissions(context, search_opt.keys(), user_api, admin_api) except exception.InvalidInput, e: - raise faults.Fault(exc.HTTPBadRequest(detail=str(e))) + raise faults.Fault(exc.HTTPBadRequest(explanation=str(e))) except exception.AdminRequired, e: - raise faults.Fault(exc.HTTPForbidden(detail=str(e))) + raise faults.Fault(exc.HTTPForbidden(explanation=str(e))) # NOTE(comstud): Making recurse_zones always be True in v1.1 search_opts['recurse_zones'] = True diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 67fb6a84e..1883ce2a5 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -60,25 +60,12 @@ class ViewBuilder(object): def _build_detail(self, inst): """Returns a detailed model of a server.""" - power_mapping = { - None: 'BUILD', - power_state.NOSTATE: 'BUILD', - power_state.RUNNING: 'ACTIVE', - power_state.BLOCKED: 'ACTIVE', - power_state.SUSPENDED: 'SUSPENDED', - power_state.PAUSED: 'PAUSED', - power_state.SHUTDOWN: 'SHUTDOWN', - power_state.SHUTOFF: 'SHUTOFF', - power_state.CRASHED: 'ERROR', - power_state.FAILED: 'ERROR', - power_state.BUILDING: 'BUILD', - } inst_dict = { 'id': inst['id'], 'name': inst['display_name'], 'addresses': self.addresses_builder.build(inst), - 'status': power_mapping[inst.get('state')]} + 'status': power_state.status_from_state(inst.get('state'))} ctxt = nova.context.get_admin_context() compute_api = nova.compute.API() diff --git a/nova/compute/api.py b/nova/compute/api.py index 6661775a5..cd2aaed96 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -669,11 +669,32 @@ class API(base.Base): return self.db.instance_get_all_by_ipv6_regexp(context, ipv6_regexp) - def _get_all_by_column(column_regexp, column): - """Get instances by matching Instance.""" + def _get_all_by_column_regexp(column_regexp, column): + """Get instances by regular expression matching + Instance. + """ return self.db.instance_get_all_by_column_regexp( context, column, column_regexp) + def _get_all_by_column(column_data, column): + """Get instances by regular expression matching + Instance. + """ + return self.db.instance_get_all_by_column( + context, column, column_data) + + def _get_all_by_flavor(flavor_id): + """Get instances by regular expression matching + Instance. + """ + try: + instance_type = self.db.instance_type_get_by_flavor_id( + context, flavor_id) + except exception.FlavorNotFound: + return [] + return self.db.instance_get_all_by_column( + context, 'instance_type_id', instance_type['id']) + # Define the search params that we will allow. This is a mapping # of the search param to tuple of (function_to_call, (function_args)) # A 'None' function means it's an optional parameter that will @@ -686,7 +707,7 @@ class API(base.Base): # v1.1 API 'changes-since': (None, None), # Mutually exclusive options - 'name': (_get_all_by_column, ('display_name',)), + 'name': (_get_all_by_column_regexp, ('display_name',)), 'reservation_id': (_get_all_by_reservation_id, ()), # Needed for EC2 API 'fixed_ip': (_get_all_by_fixed_ip, ()), @@ -694,7 +715,10 @@ class API(base.Base): 'project_id': (_get_all_by_project_id, ()), 'ip': (_get_all_by_ip, ()), 'ip6': (_get_all_by_ipv6, ()), - 'instance_name': (_get_all_by_instance_name, ())} + 'instance_name': (_get_all_by_instance_name, ()), + 'image': (_get_all_by_column, ('image_ref',)), + 'state': (_get_all_by_column, ('state',)), + 'flavor': (_get_all_by_flavor, ())} # FIXME(comstud): 'fresh' and 'changes-since' are currently not # implemented... @@ -746,13 +770,26 @@ class API(base.Base): if not search_opts.get('recurse_zones', False): return instances + new_search_opts = {} + new_search_opts.update(search_opts) + # API does state search by status, instead of the real power + # state. So if we're searching by 'state', we need to + # convert this back into 'status' + state = new_search_opts.pop('state', None) + if state: + # Might be a list.. we can only use 1. + if isinstance(state, list): + state = state[0] + new_search_opts['status'] = power_state.status_from_state( + state) + # Recurse zones. Need admin context for this. admin_context = context.elevated() children = scheduler_api.call_zone_method(admin_context, "list", errors_to_ignore=[novaclient.exceptions.NotFound], novaclient_collection_name="servers", - search_opts=search_opts) + search_opts=new_search_opts) for zone, servers in children: # 'servers' can be None if a 404 was returned by a zone diff --git a/nova/compute/power_state.py b/nova/compute/power_state.py index c468fe6b3..834ad1c0a 100644 --- a/nova/compute/power_state.py +++ b/nova/compute/power_state.py @@ -55,3 +55,32 @@ def name(code): def valid_states(): return _STATE_MAP.keys() + +_STATUS_MAP = { + None: 'BUILD', + NOSTATE: 'BUILD', + RUNNING: 'ACTIVE', + BLOCKED: 'ACTIVE', + SUSPENDED: 'SUSPENDED', + PAUSED: 'PAUSED', + SHUTDOWN: 'SHUTDOWN', + SHUTOFF: 'SHUTOFF', + CRASHED: 'ERROR', + FAILED: 'ERROR', + BUILDING: 'BUILD', +} + +def status_from_state(power_state): + """Map the power state to the server status string""" + return _STATUS_MAP[power_state] + +def states_from_status(status): + """Map the server status string to a list of power states""" + power_states = [] + for power_state, status_map in _STATUS_MAP.iteritems(): + # Skip the 'None' state + if power_state is not None: + continue + if status.lower() == status_map.lower(): + power_states.append(power_state) + return power_states diff --git a/nova/db/api.py b/nova/db/api.py index 6aa44f06b..b1b9b4544 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -520,6 +520,11 @@ def instance_get_by_fixed_ipv6(context, address): return IMPL.instance_get_by_fixed_ipv6(context, address) +def instance_get_all_by_column(context, column, column_data): + """Get all instances by exact match against the specified DB column""" + return IMPL.instance_get_all_by_column(context, column, column_data) + + def instance_get_all_by_column_regexp(context, column, column_regexp): """Get all instances by using regular expression matching against a particular DB column diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index feccba389..5209e9c8d 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1225,6 +1225,48 @@ def instance_get_by_fixed_ipv6(context, address): return result +@require_context +def instance_get_all_by_column(context, column, column_data): + """Get all instances by exact match against the specified DB column + 'column_data' can be a list. + """ + session = get_session() + + + prefix = session.query(models.Instance).\ + options(joinedload_all('fixed_ips.floating_ips')).\ + options(joinedload('virtual_interfaces')).\ + options(joinedload('security_groups')).\ + options(joinedload_all('fixed_ips.network')).\ + options(joinedload('metadata')).\ + options(joinedload('instance_type')).\ + filter_by(deleted=can_read_deleted(context)) + + if isinstance(column_data, list): + column_attr = getattr(models.Instance, column) + prefix = prefix.filter(column_attr.in_(column_data)) + else: + # Set up the dictionary for filter_by() + query_filter = {} + query_filter[column] = column_data + prefix = prefix.filter_by(**query_filter) + + if context.is_admin: + all_instances = prefix.all() + elif context.project: + all_instances = prefix.\ + filter_by(project_id=context.project_id).\ + all() + else: + all_instances = prefix.\ + filter_by(user_id=context.user_id).\ + all() + if not all_instances: + return [] + + return all_instances + + @require_context def instance_get_all_by_column_regexp(context, column, column_regexp): """Get all instances by using regular expression matching against diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index 152687083..bab58a7b1 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -84,8 +84,11 @@ class ComputeTestCase(test.TestCase): self.manager.delete_project(self.project) super(ComputeTestCase, self).tearDown() - def _create_instance(self, params={}): + def _create_instance(self, params=None): """Create a test instance""" + + if params is None: + params = {} inst = {} inst['image_ref'] = 1 inst['reservation_id'] = 'r-fakeres' @@ -825,11 +828,11 @@ class ComputeTestCase(test.TestCase): c = context.get_admin_context() instance_id1 = self._create_instance({'display_name': 'woot'}) instance_id2 = self._create_instance({ - 'display_name': 'woo', - 'id': 20}) + 'display_name': 'woo', + 'id': 20}) instance_id3 = self._create_instance({ - 'display_name': 'not-woot', - 'id': 30}) + 'display_name': 'not-woot', + 'id': 30}) instances = self.compute_api.get_all(c, search_opts={'name': 'woo.*'}) @@ -937,11 +940,11 @@ class ComputeTestCase(test.TestCase): c = context.get_admin_context() instance_id1 = self._create_instance({'server_name': 'woot'}) instance_id2 = self._create_instance({ - 'server_name': 'woo', - 'id': 20}) + 'server_name': 'woo', + 'id': 20}) instance_id3 = self._create_instance({ - 'server_name': 'not-woot', - 'id': 30}) + 'server_name': 'not-woot', + 'id': 30}) db.fixed_ip_create(c, {'address': '1.1.1.1', @@ -974,14 +977,12 @@ class ComputeTestCase(test.TestCase): instances = self.compute_api.get_all(c, search_opts={'ip': '.*\.2.+'}) self.assertEqual(len(instances), 1) - instance_ids = [instance.id for instance in instances] - self.assertTrue(instance_id2 in instance_ids) + self.assertEqual(instances[0].id, instance_id2) instances = self.compute_api.get_all(c, search_opts={'ip': '10.*'}) self.assertEqual(len(instances), 1) - instance_ids = [instance.id for instance in instances] - self.assertTrue(instance_id3 in instance_ids) + self.assertEqual(instances[0].id, instance_id3) db.instance_destroy(c, instance_id1) db.instance_destroy(c, instance_id2) @@ -1004,15 +1005,16 @@ class ComputeTestCase(test.TestCase): c = context.get_admin_context() instance_id1 = self._create_instance({'server_name': 'woot'}) instance_id2 = self._create_instance({ - 'server_name': 'woo', - 'id': 20}) + 'server_name': 'woo', + 'id': 20}) instance_id3 = self._create_instance({ - 'server_name': 'not-woot', - 'id': 30}) + 'server_name': 'not-woot', + 'id': 30}) instances = self.compute_api.get_all(c, search_opts={'ip6': 'ff.*'}) self.assertEqual(len(instances), 1) + self.assertEqual(instances[0].id, instance_id1) instance_ids = [instance.id for instance in instances] self.assertTrue(instance_id1 in instance_ids) @@ -1034,3 +1036,123 @@ class ComputeTestCase(test.TestCase): db.instance_destroy(c, instance_id1) db.instance_destroy(c, instance_id2) db.instance_destroy(c, instance_id3) + + def test_get_all_by_image(self): + """Test searching instances by image""" + + c = context.get_admin_context() + instance_id1 = self._create_instance({'image_ref': '1234'}) + instance_id2 = self._create_instance({ + 'id': 2, + 'image_ref': '4567'}) + instance_id3 = self._create_instance({ + 'id': 10, + 'image_ref': '4567'}) + + instances = self.compute_api.get_all(c, + search_opts={'image': '123'}) + self.assertEqual(len(instances), 0) + + instances = self.compute_api.get_all(c, + search_opts={'image': '1234'}) + self.assertEqual(len(instances), 1) + self.assertEqual(instances[0].id, instance_id1) + + instances = self.compute_api.get_all(c, + search_opts={'image': '4567'}) + self.assertEqual(len(instances), 2) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id2 in instance_ids) + self.assertTrue(instance_id3 in instance_ids) + + # Test passing a list as search arg + instances = self.compute_api.get_all(c, + search_opts={'image': ['1234', '4567']}) + self.assertEqual(len(instances), 3) + + db.instance_destroy(c, instance_id1) + db.instance_destroy(c, instance_id2) + db.instance_destroy(c, instance_id3) + + def test_get_all_by_flavor(self): + """Test searching instances by image""" + + c = context.get_admin_context() + instance_id1 = self._create_instance({'instance_type_id': 1}) + instance_id2 = self._create_instance({ + 'id': 2, + 'instance_type_id': 2}) + instance_id3 = self._create_instance({ + 'id': 10, + 'instance_type_id': 2}) + + # NOTE(comstud): Migrations set up the instance_types table + # for us. Therefore, we assume the following is true for + # these tests: + # instance_type_id 1 == flavor 3 + # instance_type_id 2 == flavor 1 + # instance_type_id 3 == flavor 4 + # instance_type_id 4 == flavor 5 + # instance_type_id 5 == flavor 2 + + instances = self.compute_api.get_all(c, + search_opts={'flavor': 5}) + self.assertEqual(len(instances), 0) + + instances = self.compute_api.get_all(c, + search_opts={'flavor': 99}) + self.assertEqual(len(instances), 0) + + instances = self.compute_api.get_all(c, + search_opts={'flavor': 3}) + self.assertEqual(len(instances), 1) + self.assertEqual(instances[0].id, instance_id1) + + instances = self.compute_api.get_all(c, + search_opts={'flavor': 1}) + self.assertEqual(len(instances), 2) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id2 in instance_ids) + self.assertTrue(instance_id3 in instance_ids) + + db.instance_destroy(c, instance_id1) + db.instance_destroy(c, instance_id2) + db.instance_destroy(c, instance_id3) + + def test_get_all_by_state(self): + """Test searching instances by state""" + + c = context.get_admin_context() + instance_id1 = self._create_instance({'state': power_state.SHUTDOWN}) + instance_id2 = self._create_instance({ + 'id': 2, + 'state': power_state.RUNNING}) + instance_id3 = self._create_instance({ + 'id': 10, + 'state': power_state.RUNNING}) + + instances = self.compute_api.get_all(c, + search_opts={'state': power_state.SUSPENDED}) + self.assertEqual(len(instances), 0) + + instances = self.compute_api.get_all(c, + search_opts={'state': power_state.SHUTDOWN}) + self.assertEqual(len(instances), 1) + self.assertEqual(instances[0].id, instance_id1) + + instances = self.compute_api.get_all(c, + search_opts={'state': power_state.RUNNING}) + self.assertEqual(len(instances), 2) + instance_ids = [instance.id for instance in instances] + self.assertTrue(instance_id2 in instance_ids) + self.assertTrue(instance_id3 in instance_ids) + + # Test passing a list as search arg + instances = self.compute_api.get_all(c, + search_opts={'state': [power_state.SHUTDOWN, + power_state.RUNNING]}) + self.assertEqual(len(instances), 3) + + db.instance_destroy(c, instance_id1) + db.instance_destroy(c, instance_id2) + db.instance_destroy(c, instance_id3) -- cgit From 68ca0a6e770eadf1ed56aa9d0bef14c5ca16e172 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 02:49:42 -0700 Subject: add image and flavor searching to v1.0 api fixed missing updates from cut n paste in some doc strings --- nova/api/openstack/servers.py | 3 ++- nova/compute/api.py | 8 ++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index fb1ce2529..b9347a014 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -472,7 +472,8 @@ class ControllerV10(Controller): search_opts.update(req.str_GET) user_api = ['project_id', 'fixed_ip', 'recurse_zones', - 'reservation_id', 'name', 'fresh', 'status'] + 'reservation_id', 'name', 'fresh', 'status', + 'image', 'flavor'] admin_api = ['ip', 'ip6', 'instance_name'] context = req.environ['nova.context'] diff --git a/nova/compute/api.py b/nova/compute/api.py index cd2aaed96..c4d6f8b8b 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -677,16 +677,12 @@ class API(base.Base): context, column, column_regexp) def _get_all_by_column(column_data, column): - """Get instances by regular expression matching - Instance. - """ + """Get instances by exact matching Instance.""" return self.db.instance_get_all_by_column( context, column, column_data) def _get_all_by_flavor(flavor_id): - """Get instances by regular expression matching - Instance. - """ + """Get instances by flavor ID""" try: instance_type = self.db.instance_type_get_by_flavor_id( context, flavor_id) -- cgit From a6968a100d2a2409094f7b434a88c700ebb876f3 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 02:59:03 -0700 Subject: flavor needs to be converted to int from query string value --- nova/api/openstack/servers.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index b9347a014..f470c59e3 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -494,6 +494,9 @@ class ControllerV10(Controller): # Convert recurse_zones into a boolean search_opts['recurse_zones'] = utils.bool_from_str( search_opts.get('recurse_zones', False)) + # convert flavor into an int + if 'flavor' in search_opts: + search_opts['flavor'] = int(search_opts['flavor']) return self._get_items(context, req, is_detail, search_opts=search_opts) @@ -594,6 +597,9 @@ class ControllerV11(Controller): # NOTE(comstud): Making recurse_zones always be True in v1.1 search_opts['recurse_zones'] = True + # convert flavor into an int + if 'flavor' in search_opts: + search_opts['flavor'] = int(search_opts['flavor']) return self._get_items(context, req, is_detail, search_opts=search_opts) -- cgit From bfee5105a2e557a28a605778599e99308f2a126e Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 03:02:50 -0700 Subject: typos --- nova/api/openstack/servers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index f470c59e3..9bfcf585b 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -58,7 +58,7 @@ def check_option_permissions(context, specified_options, known_options = user_api_options # Check for unknown query string params. - spec_unknown_opts = [for opt in specified_options + spec_unknown_opts = [opt for opt in specified_options if opt not in known_options] if spec_unknown_opts: unknown_opt_str = ", ".join(spec_unknown_opts) @@ -67,7 +67,7 @@ def check_option_permissions(context, specified_options, # Check for admin context for the admin commands if not context.is_admin: - spec_admin_opts = [for opt in specified_options + spec_admin_opts = [opt for opt in specified_options if opt in admin_api_options] if spec_admin_opts: admin_opt_str = ", ".join(admin_opts) -- cgit From edaeb96d6ce9c14b1f70a71c219d0353b59ed270 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 03:08:23 -0700 Subject: more typos --- nova/api/openstack/servers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 9bfcf585b..771939624 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -52,7 +52,7 @@ def check_option_permissions(context, specified_options, # We pretend we don't know about admin_api_options if the admin # API is not enabled. - if FLAGS.enable_admin_api: + if FLAGS.allow_admin_api: known_options = user_api_options + admin_api_options else: known_options = user_api_options @@ -479,7 +479,7 @@ class ControllerV10(Controller): context = req.environ['nova.context'] try: - check_option_permissions(context, search_opt.keys(), + check_option_permissions(context, search_opts.keys(), user_api, admin_api) except exception.InvalidInput: # FIXME(comstud): I refactored code in here to support -- cgit From 043cfae7737a977f7f03d75910742f741b832323 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 03:36:08 -0700 Subject: missed power_state import in api fixed reversed compare in power_state --- nova/api/openstack/servers.py | 8 +++++--- nova/compute/power_state.py | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 771939624..5a4dcbd9e 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -32,6 +32,7 @@ import nova.api.openstack.views.images import nova.api.openstack.views.servers from nova.api.openstack import wsgi import nova.api.openstack +from nova.compute import power_state from nova.scheduler import api as scheduler_api @@ -125,8 +126,9 @@ class Controller(object): if status is not None: search_opts['state'] = power_state.states_from_status(status) if len(search_opts['state']) == 0: - raise exception.InvalidInput(reason=_( - 'Invalid server status')) + reason = _('Invalid server status: %(status)s') % locals() + LOG.error(reason) + raise exception.InvalidInput(reason=reason) instance_list = self.compute_api.get_all( context, search_opts=search_opts) limited_list = self._limit_items(instance_list, req) @@ -482,7 +484,7 @@ class ControllerV10(Controller): check_option_permissions(context, search_opts.keys(), user_api, admin_api) except exception.InvalidInput: - # FIXME(comstud): I refactored code in here to support + # NOTE(comstud): I refactored code in here to support # new search options, and the original code ignored # invalid options. So, I've left it this way for now. # The v1.1 implementation will return an error in this diff --git a/nova/compute/power_state.py b/nova/compute/power_state.py index 834ad1c0a..8018c5270 100644 --- a/nova/compute/power_state.py +++ b/nova/compute/power_state.py @@ -79,7 +79,7 @@ def states_from_status(status): power_states = [] for power_state, status_map in _STATUS_MAP.iteritems(): # Skip the 'None' state - if power_state is not None: + if power_state is None: continue if status.lower() == status_map.lower(): power_states.append(power_state) -- cgit From 5a2add5c6011ce94f4727037c193274d21351cb2 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Mon, 18 Jul 2011 04:13:22 -0700 Subject: another typo --- nova/api/openstack/servers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 8df4ce31d..8c1638e21 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -588,7 +588,7 @@ class ControllerV11(Controller): context = req.environ['nova.context'] try: - check_option_permissions(context, search_opt.keys(), + check_option_permissions(context, search_opts.keys(), user_api, admin_api) except exception.InvalidInput, e: raise faults.Fault(exc.HTTPBadRequest(explanation=str(e))) -- cgit From 0b9048bc3285b86a073da9aa9327815319aaa184 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Tue, 19 Jul 2011 12:44:00 -0700 Subject: allow 'marker' and 'limit' in search options. fix log format error --- nova/api/openstack/servers.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 8c1638e21..17a3df344 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -64,8 +64,10 @@ def check_option_permissions(context, specified_options, if opt not in known_options] if spec_unknown_opts: unknown_opt_str = ", ".join(spec_unknown_opts) + LOG.error(_("Received request for unknown options " + "'%(unknown_opt_str)s'") % locals()) raise exception.InvalidInput(reason=_( - "Unknown options specified: %(unknown_opt_str)")) + "Unknown options specified: %(unknown_opt_str)s")) # Check for admin context for the admin commands if not context.is_admin: @@ -73,6 +75,9 @@ def check_option_permissions(context, specified_options, if opt in admin_api_options] if spec_admin_opts: admin_opt_str = ", ".join(admin_opts) + LOG.error(_("Received request for admin options " + "'%(admin_opt_str)s' from non-admin context") % + locals()) raise exception.AdminRequired() @@ -471,9 +476,9 @@ class ControllerV10(Controller): search_opts = {} search_opts.update(req.str_GET) - user_api = ['project_id', 'fixed_ip', 'recurse_zones', - 'reservation_id', 'name', 'fresh', 'status', - 'image', 'flavor'] + user_api = ['marker', 'limit', 'project_id', 'fixed_ip', + 'recurse_zones', 'reservation_id', 'name', 'fresh', + 'status', 'image', 'flavor'] admin_api = ['ip', 'ip6', 'instance_name'] context = req.environ['nova.context'] @@ -581,8 +586,8 @@ class ControllerV11(Controller): search_opts = {} search_opts.update(req.str_GET) - user_api = ['image', 'flavor', 'name', 'status', - 'reservation_id', 'changes-since'] + user_api = ['marker', 'limit', 'image', 'flavor', 'name', + 'status', 'reservation_id', 'changes-since'] admin_api = ['ip', 'ip6', 'instance_name'] context = req.environ['nova.context'] -- cgit From 7630aa8acc376364375ef48a3d955a7c21f50b04 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 20 Jul 2011 11:02:00 -0700 Subject: added API tests for search options fixed a couple of bugs the tests caught --- nova/api/openstack/servers.py | 5 +- nova/tests/api/openstack/fakes.py | 3 +- nova/tests/api/openstack/test_servers.py | 204 ++++++++++++++++++++++++++++++- 3 files changed, 208 insertions(+), 4 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 17a3df344..ed3f82039 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -67,14 +67,15 @@ def check_option_permissions(context, specified_options, LOG.error(_("Received request for unknown options " "'%(unknown_opt_str)s'") % locals()) raise exception.InvalidInput(reason=_( - "Unknown options specified: %(unknown_opt_str)s")) + "Unknown options specified: %(unknown_opt_str)s") % + locals()) # Check for admin context for the admin commands if not context.is_admin: spec_admin_opts = [opt for opt in specified_options if opt in admin_api_options] if spec_admin_opts: - admin_opt_str = ", ".join(admin_opts) + admin_opt_str = ", ".join(spec_admin_opts) LOG.error(_("Received request for admin options " "'%(admin_opt_str)s' from non-admin context") % locals()) diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py index 26b1de818..205a701ab 100644 --- a/nova/tests/api/openstack/fakes.py +++ b/nova/tests/api/openstack/fakes.py @@ -68,7 +68,8 @@ def fake_auth_init(self, application): @webob.dec.wsgify def fake_wsgi(self, req): - req.environ['nova.context'] = context.RequestContext(1, 1) + if 'nova.context' not in req.environ: + req.environ['nova.context'] = context.RequestContext(1, 1) return self.application diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 1577c922b..0ca42a0b5 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -724,6 +724,208 @@ class ServersTest(test.TestCase): self.assertEqual(res.status_int, 400) self.assertTrue(res.body.find('marker param') > -1) + def test_get_servers_with_bad_option_v1_0(self): + # 1.0 API ignores unknown options + def fake_get_all(compute_self, context, search_opts=None): + return [stub_instance(100)] + + self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + + req = webob.Request.blank('/v1.0/servers?unknownoption=whee') + 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_with_bad_option_v1_1(self): + req = webob.Request.blank('/v1.1/servers?unknownoption=whee') + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 400) + self.assertTrue(res.body.find( + "Unknown options specified: unknownoption") > -1) + + def test_get_servers_allows_image_v1_1(self): + def fake_get_all(compute_self, context, search_opts=None): + self.assertNotEqual(search_opts, None) + self.assertTrue('image' in search_opts) + self.assertEqual(search_opts['image'], '12345') + return [stub_instance(100)] + + self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + + req = webob.Request.blank('/v1.1/servers?image=12345') + res = req.get_response(fakes.wsgi_app()) + # The following assert will fail if either of the asserts in + # fake_get_all() fail + 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_flavor_v1_1(self): + def fake_get_all(compute_self, context, search_opts=None): + self.assertNotEqual(search_opts, None) + self.assertTrue('flavor' in search_opts) + # flavor is an integer ID + self.assertEqual(search_opts['flavor'], 12345) + return [stub_instance(100)] + + self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + + req = webob.Request.blank('/v1.1/servers?flavor=12345') + res = req.get_response(fakes.wsgi_app()) + # The following assert will fail if either of the asserts in + # fake_get_all() fail + 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_status_v1_1(self): + def fake_get_all(compute_self, context, search_opts=None): + self.assertNotEqual(search_opts, None) + self.assertTrue('state' in search_opts) + self.assertEqual(search_opts['state'], + [power_state.RUNNING, power_state.BLOCKED]) + return [stub_instance(100)] + + self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + + req = webob.Request.blank('/v1.1/servers?status=active') + res = req.get_response(fakes.wsgi_app()) + # The following assert will fail if either of the asserts in + # fake_get_all() fail + 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_name_v1_1(self): + def fake_get_all(compute_self, context, search_opts=None): + self.assertNotEqual(search_opts, None) + self.assertTrue('name' in search_opts) + self.assertEqual(search_opts['name'], 'whee.*') + return [stub_instance(100)] + + self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + + req = webob.Request.blank('/v1.1/servers?name=whee.*') + res = req.get_response(fakes.wsgi_app()) + # The following assert will fail if either of the asserts in + # fake_get_all() fail + 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_instance_name1_v1_1(self): + """Test getting servers by instance_name with admin_api + disabled + """ + FLAGS.allow_admin_api = False + req = webob.Request.blank('/v1.1/servers?instance_name=whee.*') + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 400) + self.assertTrue(res.body.find( + "Unknown options specified: instance_name") > -1) + + def test_get_servers_allows_instance_name2_v1_1(self): + """Test getting servers by instance_name with admin_api + enabled but non-admin context + """ + FLAGS.allow_admin_api = True + + context = nova.context.RequestContext('testuser', 'testproject', + is_admin=False) + req = webob.Request.blank('/v1.1/servers?instance_name=whee.*') + req.environ["nova.context"] = context + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 403) + self.assertTrue(res.body.find( + "User does not have admin privileges") > -1) + + def test_get_servers_allows_instance_name3_v1_1(self): + """Test getting servers by instance_name with admin_api + enabled and admin context + """ + FLAGS.allow_admin_api = True + + def fake_get_all(compute_self, context, search_opts=None): + self.assertNotEqual(search_opts, None) + self.assertTrue('instance_name' in search_opts) + self.assertEqual(search_opts['instance_name'], 'whee.*') + return [stub_instance(100)] + + self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + + req = webob.Request.blank('/v1.1/servers?instance_name=whee.*') + # Request admin context + context = nova.context.RequestContext('testuser', 'testproject', + is_admin=True) + req.environ["nova.context"] = context + res = req.get_response(fakes.wsgi_app()) + # The following assert will fail if either of the asserts in + # fake_get_all() fail + 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_ip_v1_1(self): + """Test getting servers by ip with admin_api enabled and + admin context + """ + FLAGS.allow_admin_api = True + + def fake_get_all(compute_self, context, search_opts=None): + self.assertNotEqual(search_opts, None) + self.assertTrue('ip' in search_opts) + self.assertEqual(search_opts['ip'], '10\..*') + return [stub_instance(100)] + + self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + + req = webob.Request.blank('/v1.1/servers?ip=10\..*') + # Request admin context + context = nova.context.RequestContext('testuser', 'testproject', + is_admin=True) + req.environ["nova.context"] = context + res = req.get_response(fakes.wsgi_app()) + # The following assert will fail if either of the asserts in + # fake_get_all() fail + 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_ip6_v1_1(self): + """Test getting servers by ip6 with admin_api enabled and + admin context + """ + FLAGS.allow_admin_api = True + + def fake_get_all(compute_self, context, search_opts=None): + self.assertNotEqual(search_opts, None) + self.assertTrue('ip6' in search_opts) + self.assertEqual(search_opts['ip6'], 'ffff.*') + return [stub_instance(100)] + + self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + + req = webob.Request.blank('/v1.1/servers?ip6=ffff.*') + # Request admin context + context = nova.context.RequestContext('testuser', 'testproject', + is_admin=True) + req.environ["nova.context"] = context + res = req.get_response(fakes.wsgi_app()) + # The following assert will fail if either of the asserts in + # fake_get_all() fail + self.assertEqual(res.status_int, 200) + servers = json.loads(res.body)['servers'] + self.assertEqual(len(servers), 1) + self.assertEqual(servers[0]['id'], 100) + def _setup_for_create_instance(self): """Shared implementation for tests below that create instance""" def instance_create(context, inst): @@ -1665,7 +1867,7 @@ class ServersTest(test.TestCase): self.assertEqual(res.status_int, 202) self.assertEqual(self.resize_called, True) - def test_resize_server_v11(self): + def test_resize_server_v1_1(self): req = webob.Request.blank('/v1.1/servers/1/action') req.content_type = 'application/json' -- cgit From b1099b43f34e41676b0508267e9ad40b2c3415e3 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 20 Jul 2011 11:32:43 -0700 Subject: ec2 fixes --- nova/api/ec2/cloud.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 76725370a..9aef5079c 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -832,9 +832,7 @@ class CloudController(object): for ec2_id in instance_id: internal_id = ec2utils.ec2_id_to_id(ec2_id) try: - instance = self.compute_api.get(context, - instance_id=internal_id, - search_opts=search_opts) + instance = self.compute_api.get(context, internal_id) except exception.NotFound: continue instances.append(instance) @@ -1021,7 +1019,7 @@ class CloudController(object): 'AvailabilityZone'), block_device_mapping=kwargs.get('block_device_mapping', {})) return self._format_run_instances(context, - instance_id=instances[0]['reservation_id']) + reservation_id=instances[0]['reservation_id']) def _do_instance(self, action, context, ec2_id): instance_id = ec2utils.ec2_id_to_id(ec2_id) -- cgit From 6ebd04c9c971e3be63cb3d6122bbca7c95004085 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 20 Jul 2011 11:43:56 -0700 Subject: test fix for renamed get_by_fixed_ip call --- nova/tests/test_metadata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/tests/test_metadata.py b/nova/tests/test_metadata.py index c862726ab..b9b14d1ea 100644 --- a/nova/tests/test_metadata.py +++ b/nova/tests/test_metadata.py @@ -52,7 +52,7 @@ class MetadataTestCase(test.TestCase): return '99.99.99.99' self.stubs.Set(api, 'instance_get', instance_get) - self.stubs.Set(api, 'fixed_ip_get_instance', instance_get) + self.stubs.Set(api, 'instance_get_by_fixed_ip', instance_get) self.stubs.Set(api, 'instance_get_floating_address', floating_get) self.app = metadatarequesthandler.MetadataRequestHandler() -- cgit From bc2a2f30e4b8ab92d6893ec333e756c92e96a932 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 20 Jul 2011 11:48:52 -0700 Subject: pep8 fixes --- nova/compute/power_state.py | 18 ++++++++++-------- nova/db/sqlalchemy/api.py | 1 - 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/nova/compute/power_state.py b/nova/compute/power_state.py index 8018c5270..bdedd8da8 100644 --- a/nova/compute/power_state.py +++ b/nova/compute/power_state.py @@ -48,14 +48,6 @@ _STATE_MAP = { BUILDING: 'building', } - -def name(code): - return _STATE_MAP[code] - - -def valid_states(): - return _STATE_MAP.keys() - _STATUS_MAP = { None: 'BUILD', NOSTATE: 'BUILD', @@ -70,10 +62,20 @@ _STATUS_MAP = { BUILDING: 'BUILD', } + +def name(code): + return _STATE_MAP[code] + + +def valid_states(): + return _STATE_MAP.keys() + + def status_from_state(power_state): """Map the power state to the server status string""" return _STATUS_MAP[power_state] + def states_from_status(status): """Map the server status string to a list of power states""" power_states = [] diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 331c58fa3..7eb724785 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1248,7 +1248,6 @@ def instance_get_all_by_column(context, column, column_data): """ session = get_session() - prefix = session.query(models.Instance).\ options(joinedload_all('fixed_ips.floating_ips')).\ options(joinedload('virtual_interfaces')).\ -- cgit From a1cb17bf98359fae760800f8467c897d859b6994 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 20 Jul 2011 12:16:23 -0700 Subject: minor fixups --- nova/api/openstack/servers.py | 14 +++++++++----- nova/compute/api.py | 7 ------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index ed3f82039..8ec74b387 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -64,11 +64,10 @@ def check_option_permissions(context, specified_options, if opt not in known_options] if spec_unknown_opts: unknown_opt_str = ", ".join(spec_unknown_opts) - LOG.error(_("Received request for unknown options " - "'%(unknown_opt_str)s'") % locals()) - raise exception.InvalidInput(reason=_( - "Unknown options specified: %(unknown_opt_str)s") % - locals()) + reason = _("Received request for unknown options " + "'%(unknown_opt_str)s'") % locals() + LOG.error(reason) + raise exception.InvalidInput(reason=reason) # Check for admin context for the admin commands if not context.is_admin: @@ -136,6 +135,11 @@ class Controller(object): reason = _('Invalid server status: %(status)s') % locals() LOG.error(reason) raise exception.InvalidInput(reason=reason) + + # Don't pass these along to compute API, if they exist. + search_opts.pop('changes-since', None) + search_opts.pop('fresh', None) + instance_list = self.compute_api.get_all( context, search_opts=search_opts) limited_list = self._limit_items(instance_list, req) diff --git a/nova/compute/api.py b/nova/compute/api.py index 6675fff52..a031f8ab5 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -744,10 +744,6 @@ class API(base.Base): # Search options are mutually exclusive known_params = { 'recurse_zones': (None, None), - # v1.0 API? - 'fresh': (None, None), - # v1.1 API - 'changes-since': (None, None), # Mutually exclusive options 'name': (_get_all_by_column_regexp, ('display_name',)), 'reservation_id': (_get_all_by_reservation_id, ()), @@ -762,9 +758,6 @@ class API(base.Base): 'state': (_get_all_by_column, ('state',)), 'flavor': (_get_all_by_flavor, ())} - # FIXME(comstud): 'fresh' and 'changes-since' are currently not - # implemented... - if search_opts is None: search_opts = {} -- cgit From 11101dfb47a7c3a37d3d3ec04f36e33fff9f59e2 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 20 Jul 2011 12:22:02 -0700 Subject: test fixes after unknown option string changes --- nova/tests/api/openstack/test_servers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 0ca42a0b5..df951c5a0 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -743,7 +743,7 @@ class ServersTest(test.TestCase): res = req.get_response(fakes.wsgi_app()) self.assertEqual(res.status_int, 400) self.assertTrue(res.body.find( - "Unknown options specified: unknownoption") > -1) + "unknown options 'unknownoption'") > -1) def test_get_servers_allows_image_v1_1(self): def fake_get_all(compute_self, context, search_opts=None): @@ -828,7 +828,7 @@ class ServersTest(test.TestCase): res = req.get_response(fakes.wsgi_app()) self.assertEqual(res.status_int, 400) self.assertTrue(res.body.find( - "Unknown options specified: instance_name") > -1) + "unknown options 'instance_name'") > -1) def test_get_servers_allows_instance_name2_v1_1(self): """Test getting servers by instance_name with admin_api -- cgit From ff1c882d7b12aa77895c549769a27ff4913b29c8 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 20 Jul 2011 12:29:42 -0700 Subject: clarify a couple comments --- nova/compute/api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index a031f8ab5..9a606add2 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -747,9 +747,9 @@ class API(base.Base): # Mutually exclusive options 'name': (_get_all_by_column_regexp, ('display_name',)), 'reservation_id': (_get_all_by_reservation_id, ()), - # Needed for EC2 API + # 'fixed_ip' needed for EC2 API 'fixed_ip': (_get_all_by_fixed_ip, ()), - # Needed for EC2 API + # 'project_id' needed for EC2 API 'project_id': (_get_all_by_project_id, ()), 'ip': (_get_all_by_ip, ()), 'ip6': (_get_all_by_ipv6, ()), -- cgit From 970b37ff9e9aef987f6e87df7d2c2e73c484e439 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 20 Jul 2011 12:35:42 -0700 Subject: missing doc strings for fixed_ip calls I renamed --- nova/db/sqlalchemy/api.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 43b9e195b..a3fc6d733 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1221,12 +1221,14 @@ def instance_get_all_by_reservation(context, reservation_id): @require_context def instance_get_by_fixed_ip(context, address): + """Return instance ref by exact match of FixedIP""" fixed_ip_ref = fixed_ip_get_by_address(context, address) return fixed_ip_ref.instance @require_context def instance_get_by_fixed_ipv6(context, address): + """Return instance ref by exact match of IPv6""" session = get_session() # convert IPv6 address to mac -- cgit From 994e219ab0b25d48b31484a43a0ac12099cf226e Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 3 Aug 2011 00:46:38 -0700 Subject: rework OS API checking of search options --- nova/api/openstack/servers.py | 181 ++++++++++++------------------- nova/tests/api/openstack/test_servers.py | 29 +++-- 2 files changed, 84 insertions(+), 126 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 2573fc48c..b028d3a40 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -41,44 +41,46 @@ LOG = logging.getLogger('nova.api.openstack.servers') FLAGS = flags.FLAGS -def check_option_permissions(context, specified_options, - user_api_options, admin_api_options): - """Check whether or not entries in 'specified_options' are valid - based on the allowed 'user_api_options' and 'admin_api_options'. +def check_admin_search_options(context, search_options, admin_api_options): + """Check for any 'admin_api_options' specified in 'search_options'. + + If admin api is not enabled, we should pretend that we know nothing + about those options.. Ie, they don't exist in user-facing API. To + achieve this, we will strip any admin options that we find from + search_options + + If admin api is enabled, we should require admin context for any + admin options specified, and return an exception in this case. + + If any exist and admin api is not enabled, strip them from + search_options (has the effect of treating them like they don't exist). + + search_options is a dictionary of "search_option": value + admin_api_options is a list + + Returns: None if options are okay. + Modifies: admin options could be stripped from search_options + Raises: exception.AdminRequired for needing admin context + """ - All inputs are lists of option names + if not FLAGS.allow_admin_api: + # Remove any admin_api_options from search_options + for option in admin_api_options: + search_options.pop(option, None) + return - Returns: exception.InvalidInput for an invalid option or - exception.AdminRequired for needing admin privs - """ + # allow_admin_api is True and admin context? Any command is okay. + if context.is_admin: + return - # We pretend we don't know about admin_api_options if the admin - # API is not enabled. - if FLAGS.allow_admin_api: - known_options = user_api_options + admin_api_options - else: - known_options = user_api_options - - # Check for unknown query string params. - spec_unknown_opts = [opt for opt in specified_options - if opt not in known_options] - if spec_unknown_opts: - unknown_opt_str = ", ".join(spec_unknown_opts) - reason = _("Received request for unknown options " - "'%(unknown_opt_str)s'") % locals() - LOG.error(reason) - raise exception.InvalidInput(reason=reason) - - # Check for admin context for the admin commands - if not context.is_admin: - spec_admin_opts = [opt for opt in specified_options - if opt in admin_api_options] - if spec_admin_opts: - admin_opt_str = ", ".join(spec_admin_opts) - LOG.error(_("Received request for admin options " - "'%(admin_opt_str)s' from non-admin context") % - locals()) - raise exception.AdminRequired() + spec_admin_opts = [opt for opt in search_options.iterkeys() + if opt in admin_api_options] + if spec_admin_opts: + admin_opt_str = ", ".join(spec_admin_opts) + LOG.error(_("Received request for admin-only search options " + "'%(admin_opt_str)s' from non-admin context") % + locals()) + raise exception.AdminRequired() class Controller(object): @@ -91,7 +93,7 @@ class Controller(object): def index(self, req): """ Returns a list of server names and ids for a given user """ try: - servers = self._items(req, is_detail=False) + servers = self._servers_from_request(req, is_detail=False) except exception.Invalid as err: return faults.Fault(exc.HTTPBadRequest(explanation=str(err))) except exception.NotFound: @@ -101,7 +103,7 @@ class Controller(object): def detail(self, req): """ Returns a list of server details for a given user """ try: - servers = self._items(req, is_detail=True) + servers = self._servers_from_request(req, is_detail=True) except exception.Invalid as err: return faults.Fault(exc.HTTPBadRequest(explanation=str(err))) except exception.NotFound as err: @@ -117,10 +119,9 @@ class Controller(object): def _action_rebuild(self, info, request, instance_id): raise NotImplementedError() - def _get_items(self, context, req, is_detail, search_opts=None): - """Returns a list of servers. - - builder - the response model builder + def _servers_search(self, context, req, is_detail, search_opts=None): + """Returns a list of servers, taking into account any search + options specified. """ if search_opts is None: @@ -147,6 +148,34 @@ class Controller(object): for inst in limited_list] return dict(servers=servers) + def _servers_from_request(self, req, is_detail): + """Returns a list of servers based on the request. + + Checks for search options and permissions on the options. + """ + + search_opts = {} + search_opts.update(req.str_GET) + + admin_api = ['ip', 'ip6', 'instance_name'] + + context = req.environ['nova.context'] + + try: + check_admin_search_options(context, search_opts, admin_api) + except exception.AdminRequired, e: + raise exc.HTTPForbidden(detail=str(e)) + + # Convert recurse_zones into a boolean + search_opts['recurse_zones'] = utils.bool_from_str( + search_opts.get('recurse_zones', False)) + # convert flavor into an int + if 'flavor' in search_opts: + search_opts['flavor'] = int(search_opts['flavor']) + + return self._servers_search(context, req, is_detail, + search_opts=search_opts) + @scheduler_api.redirect_handler def show(self, req, id): """ Returns server details by server id """ @@ -469,45 +498,6 @@ class ControllerV10(Controller): raise exc.HTTPNotFound() return webob.Response(status_int=202) - def _items(self, req, is_detail): - """Returns a list of servers based on the request. - - Checks for search options and permissions on the options. - """ - - search_opts = {} - search_opts.update(req.str_GET) - - user_api = ['marker', 'limit', 'project_id', 'fixed_ip', - 'recurse_zones', 'reservation_id', 'name', 'fresh', - 'status', 'image', 'flavor'] - admin_api = ['ip', 'ip6', 'instance_name'] - - context = req.environ['nova.context'] - - try: - check_option_permissions(context, search_opts.keys(), - user_api, admin_api) - except exception.InvalidInput: - # NOTE(comstud): I refactored code in here to support - # new search options, and the original code ignored - # invalid options. So, I've left it this way for now. - # The v1.1 implementation will return an error in this - # case.. - pass - except exception.AdminRequired, e: - raise faults.Fault(exc.HTTPForbidden(detail=str(e))) - - # Convert recurse_zones into a boolean - search_opts['recurse_zones'] = utils.bool_from_str( - search_opts.get('recurse_zones', False)) - # convert flavor into an int - if 'flavor' in search_opts: - search_opts['flavor'] = int(search_opts['flavor']) - - return self._get_items(context, req, is_detail, - search_opts=search_opts) - def _image_ref_from_req_data(self, data): return data['server']['imageId'] @@ -572,37 +562,6 @@ class ControllerV11(Controller): except exception.NotFound: raise exc.HTTPNotFound() - def _items(self, req, is_detail): - """Returns a list of servers based on the request. - - Checks for search options and permissions on the options. - """ - - search_opts = {} - search_opts.update(req.str_GET) - - user_api = ['marker', 'limit', 'image', 'flavor', 'name', - 'status', 'reservation_id', 'changes-since'] - admin_api = ['ip', 'ip6', 'instance_name'] - - context = req.environ['nova.context'] - - try: - check_option_permissions(context, search_opts.keys(), - user_api, admin_api) - except exception.InvalidInput, e: - raise faults.Fault(exc.HTTPBadRequest(explanation=str(e))) - except exception.AdminRequired, e: - raise faults.Fault(exc.HTTPForbidden(explanation=str(e))) - - # NOTE(comstud): Making recurse_zones always be True in v1.1 - search_opts['recurse_zones'] = True - # convert flavor into an int - if 'flavor' in search_opts: - search_opts['flavor'] = int(search_opts['flavor']) - return self._get_items(context, req, is_detail, - search_opts=search_opts) - def _image_ref_from_req_data(self, data): return data['server']['imageRef'] diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 821b055c4..a52940888 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1053,11 +1053,18 @@ class ServersTest(test.TestCase): self.assertEqual(servers[0]['id'], 100) def test_get_servers_with_bad_option_v1_1(self): + # 1.1 API also ignores unknown options + def fake_get_all(compute_self, context, search_opts=None): + return [stub_instance(100)] + + self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + req = webob.Request.blank('/v1.1/servers?unknownoption=whee') res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 400) - self.assertTrue(res.body.find( - "unknown options 'unknownoption'") > -1) + 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_image_v1_1(self): def fake_get_all(compute_self, context, search_opts=None): @@ -1134,17 +1141,6 @@ class ServersTest(test.TestCase): self.assertEqual(servers[0]['id'], 100) def test_get_servers_allows_instance_name1_v1_1(self): - """Test getting servers by instance_name with admin_api - disabled - """ - FLAGS.allow_admin_api = False - req = webob.Request.blank('/v1.1/servers?instance_name=whee.*') - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 400) - self.assertTrue(res.body.find( - "unknown options 'instance_name'") > -1) - - def test_get_servers_allows_instance_name2_v1_1(self): """Test getting servers by instance_name with admin_api enabled but non-admin context """ @@ -1156,10 +1152,13 @@ class ServersTest(test.TestCase): req.environ["nova.context"] = context res = req.get_response(fakes.wsgi_app()) self.assertEqual(res.status_int, 403) + print '*' * 80 + print res.body + print '*' * 80 self.assertTrue(res.body.find( "User does not have admin privileges") > -1) - def test_get_servers_allows_instance_name3_v1_1(self): + def test_get_servers_allows_instance_name2_v1_1(self): """Test getting servers by instance_name with admin_api enabled and admin context """ -- cgit From a5390a5b1cb95ca9aee6e2f99572498dd60b48e5 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 3 Aug 2011 00:53:13 -0700 Subject: remove faults.Fault wrapper on exceptions --- nova/api/openstack/servers.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index b028d3a40..6ad549c97 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -95,9 +95,9 @@ class Controller(object): try: servers = self._servers_from_request(req, is_detail=False) except exception.Invalid as err: - return faults.Fault(exc.HTTPBadRequest(explanation=str(err))) + return exc.HTTPBadRequest(explanation=str(err)) except exception.NotFound: - return faults.Fault(exc.HTTPNotFound()) + return exc.HTTPNotFound() return servers def detail(self, req): @@ -105,9 +105,9 @@ class Controller(object): try: servers = self._servers_from_request(req, is_detail=True) except exception.Invalid as err: - return faults.Fault(exc.HTTPBadRequest(explanation=str(err))) + return exc.HTTPBadRequest(explanation=str(err)) except exception.NotFound as err: - return faults.Fault(exc.HTTPNotFound()) + return exc.HTTPNotFound() return servers def _build_view(self, req, instance, is_detail=False): -- cgit From 450a21d8c1bed9cf6d1bcee9bcde7e88b9c3c6b9 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 3 Aug 2011 00:55:33 -0700 Subject: remove debug from failing test --- nova/tests/api/openstack/test_servers.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index a52940888..cdf36cabb 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1152,9 +1152,6 @@ class ServersTest(test.TestCase): req.environ["nova.context"] = context res = req.get_response(fakes.wsgi_app()) self.assertEqual(res.status_int, 403) - print '*' * 80 - print res.body - print '*' * 80 self.assertTrue(res.body.find( "User does not have admin privileges") > -1) -- cgit From b5ac286fade15a61326068e5ef0959352f885efe Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 3 Aug 2011 23:08:42 -0700 Subject: a lot of major re-work.. still things to finish up --- nova/compute/api.py | 157 ++++------------------ nova/db/api.py | 39 +----- nova/db/sqlalchemy/api.py | 328 ++++++++++++++-------------------------------- 3 files changed, 129 insertions(+), 395 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 5862b6d45..4d0654d20 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -677,155 +677,44 @@ class API(base.Base): all instances in the system. """ - def _get_all_by_reservation_id(reservation_id): - """Get instances by reservation ID""" - # reservation_id implies recurse_zones - search_opts['recurse_zones'] = True - return self.db.instance_get_all_by_reservation(context, - reservation_id) - - def _get_all_by_project_id(project_id): - """Get instances by project ID""" - return self.db.instance_get_all_by_project(context, project_id) - - def _get_all_by_fixed_ip(fixed_ip): - """Get instance by fixed IP""" - try: - instances = self.db.instance_get_by_fixed_ip(context, - fixed_ip) - except exception.FixedIpNotFound, e: - if search_opts.get('recurse_zones', False): - return [] - else: - raise - if not instances: - raise exception.FixedIpNotFoundForAddress(address=fixed_ip) - return instances - - def _get_all_by_instance_name(instance_name_regexp): - """Get instances by matching the Instance.name property""" - return self.db.instance_get_all_by_name_regexp( - context, instance_name_regexp) - - def _get_all_by_ip(ip_regexp): - """Get instances by matching IPv4 addresses""" - return self.db.instance_get_all_by_ip_regexp(context, ip_regexp) - - def _get_all_by_ipv6(ipv6_regexp): - """Get instances by matching IPv6 addresses""" - return self.db.instance_get_all_by_ipv6_regexp(context, - ipv6_regexp) - - def _get_all_by_column_regexp(column_regexp, column): - """Get instances by regular expression matching - Instance. - """ - return self.db.instance_get_all_by_column_regexp( - context, column, column_regexp) - - def _get_all_by_column(column_data, column): - """Get instances by exact matching Instance.""" - return self.db.instance_get_all_by_column( - context, column, column_data) - - def _get_all_by_flavor(flavor_id): - """Get instances by flavor ID""" - try: - instance_type = self.db.instance_type_get_by_flavor_id( - context, flavor_id) - except exception.FlavorNotFound: - return [] - return self.db.instance_get_all_by_column( - context, 'instance_type_id', instance_type['id']) - - # Define the search params that we will allow. This is a mapping - # of the search param to tuple of (function_to_call, (function_args)) - # A 'None' function means it's an optional parameter that will - # influence the search results, but itself is not a search option. - # Search options are mutually exclusive - known_params = { - 'recurse_zones': (None, None), - # Mutually exclusive options - 'name': (_get_all_by_column_regexp, ('display_name',)), - 'reservation_id': (_get_all_by_reservation_id, ()), - # 'fixed_ip' needed for EC2 API - 'fixed_ip': (_get_all_by_fixed_ip, ()), - # 'project_id' needed for EC2 API - 'project_id': (_get_all_by_project_id, ()), - 'ip': (_get_all_by_ip, ()), - 'ip6': (_get_all_by_ipv6, ()), - 'instance_name': (_get_all_by_instance_name, ()), - 'image': (_get_all_by_column, ('image_ref',)), - 'state': (_get_all_by_column, ('state',)), - 'flavor': (_get_all_by_flavor, ())} - if search_opts is None: search_opts = {} LOG.debug(_("Searching by: %s") % str(search_opts)) - # Mutually exclusive serach options are any options that have - # a function to call. Raise an exception if more than 1 is - # specified... - # NOTE(comstud): Ignore unknown options. The OS API will - # do it's own verification on options.. - found_opts = [opt for opt in search_opts.iterkeys() - if opt in known_params and \ - known_params[opt][0] is not None] - if len(found_opts) > 1: - found_opt_str = ", ".join(found_opts) - msg = _("More than 1 mutually exclusive " - "search option specified: %(found_opt_str)s") \ - % locals() - LOG.error(msg) - raise exception.InvalidInput(reason=msg) - - # Found a search option? - if found_opts: - found_opt = found_opts[0] - f, f_args = known_params[found_opt] - instances = f(search_opts[found_opt], *f_args) - # Nope. Return all instances if the request is in admin context.. - elif context.is_admin: - instances = self.db.instance_get_all(context) - # Nope. Return all instances for the user/project - else: - if context.project_id: - instances = self.db.instance_get_all_by_project( - context, context.project_id) + # Fixups for the DB call + filters = search_opts.copy() + if 'image' in filters: + filters['image_ref'] = filters['image'] + del filters['image'] + if 'flavor' in filters: + flavor_id = int(filters['flavor']) + try: + instance_type = self.db.instance_type_get_by_flavor_id( + context, flavor_id) + except exception.FlavorNotFound: + pass else: - instances = self.db.instance_get_all_by_user( - context, context.user_id) + filters['instance_type_id'] = instance_type['id'] + del filters['flavor'] + + recurse_zones = filters.pop('recurse_zones', False) + if 'reservation_id' in filters: + recurse_zones = True - # Convert any responses into a list of instances - if instances is None: - instances = [] - elif not isinstance(instances, list): - instances = [instances] + instances = self.db.instance_get_all_by_filters(context, filters) - if not search_opts.get('recurse_zones', False): + if not recurse_zones: return instances - new_search_opts = {} - new_search_opts.update(search_opts) - # API does state search by status, instead of the real power - # state. So if we're searching by 'state', we need to - # convert this back into 'status' - state = new_search_opts.pop('state', None) - if state: - # Might be a list.. we can only use 1. - if isinstance(state, list): - state = state[0] - new_search_opts['status'] = power_state.status_from_state( - state) - - # Recurse zones. Need admin context for this. + # Recurse zones. Need admin context for this. Send along + # the un-modified search options we received.. admin_context = context.elevated() children = scheduler_api.call_zone_method(admin_context, "list", errors_to_ignore=[novaclient.exceptions.NotFound], novaclient_collection_name="servers", - search_opts=new_search_opts) + search_opts=search_opts) for zone, servers in children: # 'servers' can be None if a 404 was returned by a zone diff --git a/nova/db/api.py b/nova/db/api.py index 22dd8a4b4..c7d5420e1 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -491,6 +491,10 @@ def instance_get_all(context): return IMPL.instance_get_all(context) +def instance_get_all_by-filters(context, filters): + """Get all instances that match all filters.""" + return IMPL.instance_get_all_by_filters(context, filters) + def instance_get_active_by_window(context, begin, end=None): """Get instances active during a certain time window.""" return IMPL.instance_get_active_by_window(context, begin, end) @@ -526,41 +530,6 @@ def instance_get_by_fixed_ipv6(context, address): return IMPL.instance_get_by_fixed_ipv6(context, address) -def instance_get_all_by_column(context, column, column_data): - """Get all instances by exact match against the specified DB column""" - return IMPL.instance_get_all_by_column(context, column, column_data) - - -def instance_get_all_by_column_regexp(context, column, column_regexp): - """Get all instances by using regular expression matching against - a particular DB column - """ - return IMPL.instance_get_all_by_column_regexp(context, - column, - column_regexp) - - -def instance_get_all_by_name_regexp(context, name_regexp): - """Get all instances by using regular expression matching against - its name - """ - return IMPL.instance_get_all_by_name_regexp(context, name_regexp) - - -def instance_get_all_by_ip_regexp(context, ip_regexp): - """Get all instances by using regular expression matching against - Floating and Fixed IP Addresses - """ - return IMPL.instance_get_all_by_ip_regexp(context, ip_regexp) - - -def instance_get_all_by_ipv6_regexp(context, ipv6_regexp): - """Get all instances by using regular expression matching against - IPv6 Addresses - """ - return IMPL.instance_get_all_by_ipv6_regexp(context, ipv6_regexp) - - def instance_get_fixed_addresses(context, instance_id): """Get the fixed ip address of an instance.""" return IMPL.instance_get_fixed_addresses(context, instance_id) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index e4250a6cb..25a486b9c 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1147,6 +1147,108 @@ def instance_get_all(context): all() +@require_context +def instance_get_all_by_filters(context, filters): + """Return instances the match all filters""" + + def _filter_by_ipv6(instance, filter_re): + for interface in instance['virtual_interfaces']: + fixed_ipv6 = interface.get('fixed_ipv6') + if fixed_ipv6 and filter_re.match(fixed_ipv6): + return True + return False + + def _filter_by_ip(instance, filter_re): + for interface in instance['virtual_interfaces']: + for fixed_ip in interface['fixed_ips']: + if not fixed_ip or not fixed_ip['address']: + continue + if filter_re.match(fixed_ip['address']): + return True + for floating_ip in fixed_ip.get('floating_ips', []): + if not floating_ip or not floating_ip['address']: + continue + if filter_re.match(floating_ip['address']): + return True + return False + + def _filter_by_display_name(instance, filter_re): + if filter_re.match(instance.display_name): + return True + return False + + def _filter_by_column(instance, filter_name, filter_re): + try: + v = getattr(instance, filter_name) + except AttributeError: + return True + if v and filter_re.match(str(v)): + return True + return False + + session = get_session() + query_prefix = session.query(models.Instance).\ + options(joinedload_all('fixed_ips.floating_ips')).\ + options(joinedload_all('virtual_interfaces.network')).\ + options(joinedload_all( + 'virtual_interfaces.fixed_ips.floating_ips')).\ + options(joinedload('security_groups')).\ + options(joinedload_all('fixed_ips.network')).\ + options(joinedload('metadata')).\ + options(joinedload('instance_type')).\ + filter_by(deleted=can_read_deleted(context)) + + filters = filters.copy() + + if not context.is_admin: + # If we're not admin context, add appropriate filter.. + if context.project_id: + filters['project_id'] = context.project_id + else: + filters['user_id'] = context.user_id + + # Filters that we can do along with the SQL query... + query_filter_funcs = { + 'project_id': lambda query, value: query.filter_by( + project_id=value), + 'user_id': lambda query, value: query.filter_by( + user_id=value), + 'reservation_id': lambda query, value: query.filter_by( + reservation_id=value), + 'state': lambda query, value: query.filter_by(state=value)} + + query_filters = [key for key in filters.iterkeys() + if key in query_filter_funcs] + + for filter_name in query_filters: + query_prefix = query_filter_funcs[filter_name](query_prefix, + filters[filter_name]) + # Remove this from filters, so it doesn't get tried below + del filters[filter_name] + + instances = query_prefix.all() + + if not instances: + return [] + + # Now filter on everything else for regexp matching.. + filter_funcs = {'ip6': _filter_by_ipv6, + 'ip': _filter_by_ip, + 'name': _filter_by_display_name} + + for filter_name in filters.iterkeys(): + filter_func = filter_funcs.get(filter_name, None) + filter_re = re.compile(filters[filter_name]) + if filter_func: + filter_l = lambda instance: filter_func(instance, filter_re) + else: + filter_l = lambda instance: _filter_by_column(instance, + filter_name, filter_re) + instances = filter(filter_l, instances) + + return instances + + @require_admin_context def instance_get_active_by_window(context, begin, end=None): """Return instances that were continuously active over the given window""" @@ -1259,232 +1361,6 @@ def instance_get_by_fixed_ipv6(context, address): return result -@require_context -def instance_get_all_by_column(context, column, column_data): - """Get all instances by exact match against the specified DB column - 'column_data' can be a list. - """ - session = get_session() - - prefix = session.query(models.Instance).\ - options(joinedload_all('fixed_ips.floating_ips')).\ - options(joinedload('virtual_interfaces')).\ - options(joinedload('security_groups')).\ - options(joinedload_all('fixed_ips.network')).\ - options(joinedload('metadata')).\ - options(joinedload('instance_type')).\ - filter_by(deleted=can_read_deleted(context)) - - if isinstance(column_data, list): - column_attr = getattr(models.Instance, column) - prefix = prefix.filter(column_attr.in_(column_data)) - else: - # Set up the dictionary for filter_by() - query_filter = {} - query_filter[column] = column_data - prefix = prefix.filter_by(**query_filter) - - if context.is_admin: - all_instances = prefix.all() - elif context.project: - all_instances = prefix.\ - filter_by(project_id=context.project_id).\ - all() - else: - all_instances = prefix.\ - filter_by(user_id=context.user_id).\ - all() - if not all_instances: - return [] - - return all_instances - - -@require_context -def instance_get_all_by_column_regexp(context, column, column_regexp): - """Get all instances by using regular expression matching against - a particular DB column - """ - session = get_session() - - # MySQL 'regexp' is not portable, so we must do our own matching. - # First... grab all Instances. - prefix = session.query(models.Instance).\ - options(joinedload_all('fixed_ips.floating_ips')).\ - options(joinedload('virtual_interfaces')).\ - options(joinedload('security_groups')).\ - options(joinedload_all('fixed_ips.network')).\ - options(joinedload('metadata')).\ - options(joinedload('instance_type')).\ - filter_by(deleted=can_read_deleted(context)) - - if context.is_admin: - all_instances = prefix.all() - elif context.project: - all_instances = prefix.\ - filter_by(project_id=context.project_id).\ - all() - else: - all_instances = prefix.\ - filter_by(user_id=context.user_id).\ - all() - if not all_instances: - return [] - # Now do the regexp matching - compiled_regexp = re.compile(column_regexp) - instances = [] - for instance in all_instances: - v = getattr(instance, column) - if v and compiled_regexp.match(v): - instances.append(instance) - return instances - - -@require_context -def instance_get_all_by_name_regexp(context, name_regexp): - """Get all instances by using regular expression matching against - its name - """ - - session = get_session() - - # MySQL 'regexp' is not portable, so we must do our own matching. - # First... grab all Instances. - prefix = session.query(models.Instance).\ - options(joinedload_all('fixed_ips.floating_ips')).\ - options(joinedload('virtual_interfaces')).\ - options(joinedload('security_groups')).\ - options(joinedload_all('fixed_ips.network')).\ - options(joinedload('metadata')).\ - options(joinedload('instance_type')).\ - filter_by(deleted=can_read_deleted(context)) - - if context.is_admin: - all_instances = prefix.all() - elif context.project: - all_instances = prefix.\ - filter_by(project_id=context.project_id).\ - all() - else: - all_instances = prefix.\ - filter_by(user_id=context.user_id).\ - all() - if not all_instances: - return [] - # Now do the regexp matching - compiled_regexp = re.compile(name_regexp) - return [instance for instance in all_instances - if compiled_regexp.match(instance.name)] - - -@require_context -def instance_get_all_by_ip_regexp(context, ip_regexp): - """Get all instances by using regular expression matching against - Floating and Fixed IP Addresses - """ - session = get_session() - - # Query both FixedIp and FloatingIp tables to get matches. - # Since someone could theoretically search for something that matches - # instances in both tables... we need to use a dictionary keyed - # on instance ID to make sure we return only 1. We can't key off - # of 'instance' because it's just a reference and will be different - # addresses even though they might point to the same instance ID. - instances = {} - - fixed_ips = session.query(models.FixedIp).\ - options(joinedload_all('instance.fixed_ips.floating_ips')).\ - options(joinedload('instance.virtual_interfaces')).\ - options(joinedload('instance.security_groups')).\ - options(joinedload_all('instance.fixed_ips.network')).\ - options(joinedload('instance.metadata')).\ - options(joinedload('instance.instance_type')).\ - filter_by(deleted=can_read_deleted(context)).\ - all() - floating_ips = session.query(models.FloatingIp).\ - options(joinedload_all( - 'fixed_ip.instance.fixed_ips.floating_ips')).\ - options(joinedload('fixed_ip.instance.virtual_interfaces')).\ - options(joinedload('fixed_ip.instance.security_groups')).\ - options(joinedload_all( - 'fixed_ip.instance.fixed_ips.network')).\ - options(joinedload('fixed_ip.instance.metadata')).\ - options(joinedload('fixed_ip.instance.instance_type')).\ - filter_by(deleted=can_read_deleted(context)).\ - all() - - if fixed_ips is None: - fixed_ips = [] - if floating_ips is None: - floating_ips = [] - - compiled_regexp = re.compile(ip_regexp) - instances = {} - - # Now do the regexp matching - for fixed_ip in fixed_ips: - if fixed_ip.instance and compiled_regexp.match(fixed_ip.address): - instances[fixed_ip.instance.uuid] = fixed_ip.instance - for floating_ip in floating_ips: - fixed_ip = floating_ip.fixed_ip - if fixed_ip and fixed_ip.instance and\ - compiled_regexp.match(floating_ip.address): - instances[fixed_ip.instance.uuid] = fixed_ip.instance - - if context.is_admin: - return instances.values() - elif context.project: - return [instance for instance in instances.values() - if instance.project_id == context.project_id] - else: - return [instance for instance in instances.values() - if instance.user_id == context.user_id] - - return instances.values() - - -@require_context -def instance_get_all_by_ipv6_regexp(context, ipv6_regexp): - """Get all instances by using regular expression matching against - IPv6 Addresses - """ - - session = get_session() - with session.begin(): - prefix = session.query(models.Instance).\ - options(joinedload_all('fixed_ips.floating_ips')).\ - options(joinedload('virtual_interfaces')).\ - options(joinedload('security_groups')).\ - options(joinedload_all('fixed_ips.network')).\ - options(joinedload('metadata')).\ - options(joinedload('instance_type')).\ - filter_by(deleted=can_read_deleted(context)) - - if context.is_admin: - all_instances = prefix.all() - elif context.project: - all_instances = prefix.\ - filter_by(project_id=context.project_id).\ - all() - else: - all_instances = prefix.\ - filter_by(user_id=context.user_id).\ - all() - - if not all_instances: - return [] - - instances = [] - compiled_regexp = re.compile(ipv6_regexp) - for instance in all_instances: - ipv6_addrs = _ipv6_get_by_instance_ref(context, instance) - for ipv6 in ipv6_addrs: - if compiled_regexp.match(ipv6): - instances.append(instance) - break - return instances - - @require_admin_context def instance_get_project_vpn(context, project_id): session = get_session() -- cgit From c0851f2ec5be12c43cc96367e22220d25589e4ae Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 01:36:12 -0700 Subject: cleanup checking of options in the API before calling compute_api's get_all() --- nova/api/openstack/servers.py | 104 ++++++++++++++++++------------------------ nova/db/sqlalchemy/api.py | 7 +-- 2 files changed, 49 insertions(+), 62 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 965cf0bfc..e3e829e81 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -44,55 +44,25 @@ LOG = logging.getLogger('nova.api.openstack.servers') FLAGS = flags.FLAGS -def check_admin_search_options(context, search_options, admin_api_options): - """Check for any 'admin_api_options' specified in 'search_options'. - - If admin api is not enabled, we should pretend that we know nothing - about those options.. Ie, they don't exist in user-facing API. To - achieve this, we will strip any admin options that we find from - search_options - - If admin api is enabled, we should require admin context for any - admin options specified, and return an exception in this case. - - If any exist and admin api is not enabled, strip them from - search_options (has the effect of treating them like they don't exist). - - search_options is a dictionary of "search_option": value - admin_api_options is a list - - Returns: None if options are okay. - Modifies: admin options could be stripped from search_options - Raises: exception.AdminRequired for needing admin context - """ - - if not FLAGS.allow_admin_api: - # Remove any admin_api_options from search_options - for option in admin_api_options: - search_options.pop(option, None) - return - - # allow_admin_api is True and admin context? Any command is okay. - if context.is_admin: - return - - spec_admin_opts = [opt for opt in search_options.iterkeys() - if opt in admin_api_options] - if spec_admin_opts: - admin_opt_str = ", ".join(spec_admin_opts) - LOG.error(_("Received request for admin-only search options " - "'%(admin_opt_str)s' from non-admin context") % - locals()) - raise exception.AdminRequired() - - class Controller(object): - """ The Server API controller for the OpenStack API """ + """ The Server API base controller class for the OpenStack API """ + + servers_search_options = [] def __init__(self): self.compute_api = compute.API() self.helper = helper.CreateInstanceHelper(self) + def _check_servers_options(self, search_options): + if FLAGS.allow_admin_api and context.is_admin: + # Allow all options + return + # Otherwise, strip out all unknown options + unknown_options = [opt for opt in search_options + if opt not in self.servers_search_options] + for opt in unknown_options: + search_options.pop(opt, None) + def index(self, req): """ Returns a list of server names and ids for a given user """ try: @@ -131,21 +101,38 @@ class Controller(object): search_opts = {} # If search by 'status', we need to convert it to 'state' - # If the status is unknown, bail - status = search_opts.pop('status', None) - if status is not None: + # If the status is unknown, bail. + # Leave 'state' in search_opts so compute can pass it on to + # child zones.. + if 'status' in search_opts: + status = search_opts['status'] search_opts['state'] = power_state.states_from_status(status) if len(search_opts['state']) == 0: reason = _('Invalid server status: %(status)s') % locals() LOG.error(reason) raise exception.InvalidInput(reason=reason) - # Don't pass these along to compute API, if they exist. - search_opts.pop('changes-since', None) - search_opts.pop('fresh', None) + # 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. + # ... Unless 'changes-since' is specified, because 'changes-since' + # 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 + 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? + limited_list = self._limit_items(instance_list, req) servers = [self._build_view(req, inst, is_detail)['server'] for inst in limited_list] @@ -160,21 +147,12 @@ class Controller(object): search_opts = {} search_opts.update(req.str_GET) - admin_api = ['ip', 'ip6', 'instance_name'] - context = req.environ['nova.context'] - - try: - check_admin_search_options(context, search_opts, admin_api) - except exception.AdminRequired, e: - raise exc.HTTPForbidden(detail=str(e)) + self._check_servers_options(context, search_opts) # Convert recurse_zones into a boolean search_opts['recurse_zones'] = utils.bool_from_str( search_opts.get('recurse_zones', False)) - # convert flavor into an int - if 'flavor' in search_opts: - search_opts['flavor'] = int(search_opts['flavor']) return self._servers_search(context, req, is_detail, search_opts=search_opts) @@ -581,6 +559,10 @@ class Controller(object): class ControllerV10(Controller): + """v1.0 OpenStack API controller""" + + servers_search_options = ["reservation_id", "fixed_ip", + "name", "recurse_zones"] @scheduler_api.redirect_handler def delete(self, req, id): @@ -645,6 +627,10 @@ class ControllerV10(Controller): class ControllerV11(Controller): + """v1.1 OpenStack API controller""" + + servers_search_options = ["reservation_id", "name", "recurse_zones", + "status", "image", "flavor", "changes-since"] @scheduler_api.redirect_handler def delete(self, req, id): diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 25a486b9c..04c9273b0 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1149,7 +1149,9 @@ def instance_get_all(context): @require_context def instance_get_all_by_filters(context, filters): - """Return instances the match all filters""" + """Return instances the match all filters. Deleted instances + will be returned by default, unless there's a filter that says + otherwise""" def _filter_by_ipv6(instance, filter_re): for interface in instance['virtual_interfaces']: @@ -1195,8 +1197,7 @@ def instance_get_all_by_filters(context, filters): options(joinedload('security_groups')).\ options(joinedload_all('fixed_ips.network')).\ options(joinedload('metadata')).\ - options(joinedload('instance_type')).\ - filter_by(deleted=can_read_deleted(context)) + options(joinedload('instance_type')) filters = filters.copy() -- cgit From b9ecf869761ee0506872b0d44d93d453be4c3477 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 01:45:42 -0700 Subject: typos --- nova/compute/api.py | 4 ++-- nova/db/api.py | 2 +- nova/db/sqlalchemy/api.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 4d0654d20..4d577b578 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -725,8 +725,8 @@ class API(base.Base): server._info['_is_precooked'] = True instances.append(server._info) - # Fixed IP returns a FixedIpNotFound when an instance is not - # found... + # fixed_ip searching should return a FixedIpNotFound exception + # when an instance is not found... fixed_ip = search_opts.get('fixed_ip', None) if fixed_ip and not instances: raise exception.FixedIpNotFoundForAddress(address=fixed_ip) diff --git a/nova/db/api.py b/nova/db/api.py index c7d5420e1..23fac9921 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -491,7 +491,7 @@ 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): """Get all instances that match all filters.""" return IMPL.instance_get_all_by_filters(context, filters) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 04c9273b0..55c804ae9 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1149,7 +1149,7 @@ def instance_get_all(context): @require_context def instance_get_all_by_filters(context, filters): - """Return instances the match all filters. Deleted instances + """Return instances that match all filters. Deleted instances will be returned by default, unless there's a filter that says otherwise""" -- cgit From b1b919d42d8c359fc9ae981b44466d269fc688a6 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 13:59:57 -0700 Subject: test fixes and typos --- nova/api/openstack/common.py | 33 +++++++++++++++++++++ nova/api/openstack/servers.py | 9 +++--- nova/api/openstack/views/servers.py | 3 +- nova/compute/power_state.py | 31 -------------------- nova/db/sqlalchemy/api.py | 49 +++++++++++++++++++------------- nova/tests/api/openstack/fakes.py | 2 -- nova/tests/api/openstack/test_servers.py | 13 +++++---- 7 files changed, 75 insertions(+), 65 deletions(-) diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index efa4ab385..bbf46261b 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -25,6 +25,7 @@ from nova import exception from nova import flags from nova import log as logging from nova.api.openstack import wsgi +from nova.compute import power_state as compute_power_state LOG = logging.getLogger('nova.api.openstack.common') @@ -35,6 +36,38 @@ XML_NS_V10 = 'http://docs.rackspacecloud.com/servers/api/v1.0' XML_NS_V11 = 'http://docs.openstack.org/compute/api/v1.1' +_STATUS_MAP = { + None: 'BUILD', + compute_power_state.NOSTATE: 'BUILD', + compute_power_state.RUNNING: 'ACTIVE', + compute_power_state.BLOCKED: 'ACTIVE', + compute_power_state.SUSPENDED: 'SUSPENDED', + compute_power_state.PAUSED: 'PAUSED', + compute_power_state.SHUTDOWN: 'SHUTDOWN', + compute_power_state.SHUTOFF: 'SHUTOFF', + compute_power_state.CRASHED: 'ERROR', + compute_power_state.FAILED: 'ERROR', + compute_power_state.BUILDING: 'BUILD', +} + + +def status_from_power_state(power_state): + """Map the power state to the server status string""" + return _STATUS_MAP[power_state] + + +def power_states_from_status(status): + """Map the server status string to a list of power states""" + power_states = [] + for power_state, status_map in _STATUS_MAP.iteritems(): + # Skip the 'None' state + if power_state is None: + continue + if status.lower() == status_map.lower(): + power_states.append(power_state) + return power_states + + def get_pagination_params(request): """Return marker, limit tuple from request. diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index e3e829e81..c842fcc01 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -31,7 +31,6 @@ from nova.api.openstack import create_instance_helper as helper from nova.api.openstack import ips from nova.api.openstack import wsgi from nova.compute import instance_types -from nova.compute import power_state from nova.scheduler import api as scheduler_api import nova.api.openstack import nova.api.openstack.views.addresses @@ -53,7 +52,7 @@ class Controller(object): self.compute_api = compute.API() self.helper = helper.CreateInstanceHelper(self) - def _check_servers_options(self, search_options): + def _check_servers_options(self, context, search_options): if FLAGS.allow_admin_api and context.is_admin: # Allow all options return @@ -106,7 +105,7 @@ class Controller(object): # child zones.. if 'status' in search_opts: status = search_opts['status'] - search_opts['state'] = power_state.states_from_status(status) + search_opts['state'] = common.power_states_from_status(status) if len(search_opts['state']) == 0: reason = _('Invalid server status: %(status)s') % locals() LOG.error(reason) @@ -122,10 +121,10 @@ class Controller(object): # Admin hasn't specified deleted filter if 'changes-since' not in search_opts: # No 'changes-since', so we need to find non-deleted servers - search_opts['deleted'] = '^False$' + search_opts['deleted'] = False else: # This is the default, but just in case.. - search_opts['deleted'] = '^True$' + search_opts['deleted'] = True instance_list = self.compute_api.get_all( context, search_opts=search_opts) diff --git a/nova/api/openstack/views/servers.py b/nova/api/openstack/views/servers.py index 64be00f86..8222f6766 100644 --- a/nova/api/openstack/views/servers.py +++ b/nova/api/openstack/views/servers.py @@ -20,7 +20,6 @@ import hashlib import os from nova import exception -from nova.compute import power_state import nova.compute import nova.context from nova.api.openstack import common @@ -65,7 +64,7 @@ class ViewBuilder(object): inst_dict = { 'id': inst['id'], 'name': inst['display_name'], - 'status': power_state.status_from_state(inst.get('state'))} + 'status': common.status_from_power_state(inst.get('state'))} ctxt = nova.context.get_admin_context() compute_api = nova.compute.API() diff --git a/nova/compute/power_state.py b/nova/compute/power_state.py index bdedd8da8..c468fe6b3 100644 --- a/nova/compute/power_state.py +++ b/nova/compute/power_state.py @@ -48,20 +48,6 @@ _STATE_MAP = { BUILDING: 'building', } -_STATUS_MAP = { - None: 'BUILD', - NOSTATE: 'BUILD', - RUNNING: 'ACTIVE', - BLOCKED: 'ACTIVE', - SUSPENDED: 'SUSPENDED', - PAUSED: 'PAUSED', - SHUTDOWN: 'SHUTDOWN', - SHUTOFF: 'SHUTOFF', - CRASHED: 'ERROR', - FAILED: 'ERROR', - BUILDING: 'BUILD', -} - def name(code): return _STATE_MAP[code] @@ -69,20 +55,3 @@ def name(code): def valid_states(): return _STATE_MAP.keys() - - -def status_from_state(power_state): - """Map the power state to the server status string""" - return _STATUS_MAP[power_state] - - -def states_from_status(status): - """Map the server status string to a list of power states""" - power_states = [] - for power_state, status_map in _STATUS_MAP.iteritems(): - # Skip the 'None' state - if power_state is None: - continue - if status.lower() == status_map.lower(): - power_states.append(power_state) - return power_states diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 55c804ae9..f8920e62c 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1153,14 +1153,14 @@ def instance_get_all_by_filters(context, filters): will be returned by default, unless there's a filter that says otherwise""" - def _filter_by_ipv6(instance, filter_re): + def _regexp_filter_by_ipv6(instance, filter_re): for interface in instance['virtual_interfaces']: fixed_ipv6 = interface.get('fixed_ipv6') if fixed_ipv6 and filter_re.match(fixed_ipv6): return True return False - def _filter_by_ip(instance, filter_re): + def _regexp_filter_by_ip(instance, filter_re): for interface in instance['virtual_interfaces']: for fixed_ip in interface['fixed_ips']: if not fixed_ip or not fixed_ip['address']: @@ -1174,12 +1174,12 @@ def instance_get_all_by_filters(context, filters): return True return False - def _filter_by_display_name(instance, filter_re): + def _regexp_filter_by_display_name(instance, filter_re): if filter_re.match(instance.display_name): return True return False - def _filter_by_column(instance, filter_name, filter_re): + def _regexp_filter_by_column(instance, filter_name, filter_re): try: v = getattr(instance, filter_name) except AttributeError: @@ -1188,6 +1188,18 @@ def instance_get_all_by_filters(context, filters): return True return False + def _exact_match_filter(query, column, value): + """Do exact match against a column. value to match can be a list + so you can match any value in the list. + """ + if isinstance(value, list): + column_attr = getattr(models.Instance, column) + return query.filter(column_attr.in_(value)) + else: + filter_dict = {} + filter_dict[column] = value + return query.filter_by(**filter_dict) + session = get_session() query_prefix = session.query(models.Instance).\ options(joinedload_all('fixed_ips.floating_ips')).\ @@ -1208,21 +1220,16 @@ def instance_get_all_by_filters(context, filters): else: filters['user_id'] = context.user_id - # Filters that we can do along with the SQL query... - query_filter_funcs = { - 'project_id': lambda query, value: query.filter_by( - project_id=value), - 'user_id': lambda query, value: query.filter_by( - user_id=value), - 'reservation_id': lambda query, value: query.filter_by( - reservation_id=value), - 'state': lambda query, value: query.filter_by(state=value)} + # Filters for exact matches that we can do along with the SQL query... + # For other filters that don't match this, we will do regexp matching + exact_match_filter_names = ['project_id', 'user_id', 'image_ref', + 'state', 'instance_type_id', 'deleted'] query_filters = [key for key in filters.iterkeys() - if key in query_filter_funcs] + if key in exact_match_filter_names] for filter_name in query_filters: - query_prefix = query_filter_funcs[filter_name](query_prefix, + query_prefix = _exact_match_filter(query_prefix, filter_name, filters[filter_name]) # Remove this from filters, so it doesn't get tried below del filters[filter_name] @@ -1233,17 +1240,19 @@ def instance_get_all_by_filters(context, filters): return [] # Now filter on everything else for regexp matching.. - filter_funcs = {'ip6': _filter_by_ipv6, - 'ip': _filter_by_ip, - 'name': _filter_by_display_name} + # For filters not in the list, we'll attempt to use the filter_name + # as a column name in Instance.. + regexp_filter_funcs = {'ip6': _regexp_filter_by_ipv6, + 'ip': _regexp_filter_by_ip, + 'name': _regexp_filter_by_display_name} for filter_name in filters.iterkeys(): - filter_func = filter_funcs.get(filter_name, None) + filter_func = regexp_filter_funcs.get(filter_name, None) filter_re = re.compile(filters[filter_name]) if filter_func: filter_l = lambda instance: filter_func(instance, filter_re) else: - filter_l = lambda instance: _filter_by_column(instance, + filter_l = lambda instance: _regexp_filter_by_column(instance, filter_name, filter_re) instances = filter(filter_l, instances) diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py index 33419d359..a67a28a4e 100644 --- a/nova/tests/api/openstack/fakes.py +++ b/nova/tests/api/openstack/fakes.py @@ -68,8 +68,6 @@ def fake_auth_init(self, application): @webob.dec.wsgify def fake_wsgi(self, req): - if 'nova.context' not in req.environ: - req.environ['nova.context'] = context.RequestContext(1, 1) return self.application diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index cc439f024..2ae45b791 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1149,6 +1149,7 @@ class ServersTest(test.TestCase): return [stub_instance(100)] self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + self.flags(allow_admin_api=False) req = webob.Request.blank('/v1.1/servers?image=12345') res = req.get_response(fakes.wsgi_app()) @@ -1168,6 +1169,7 @@ class ServersTest(test.TestCase): return [stub_instance(100)] self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + self.flags(allow_admin_api=False) req = webob.Request.blank('/v1.1/servers?flavor=12345') res = req.get_response(fakes.wsgi_app()) @@ -1187,6 +1189,7 @@ class ServersTest(test.TestCase): return [stub_instance(100)] self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + self.flags(allow_admin_api=False) req = webob.Request.blank('/v1.1/servers?status=active') res = req.get_response(fakes.wsgi_app()) @@ -1205,6 +1208,7 @@ class ServersTest(test.TestCase): return [stub_instance(100)] self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + self.flags(allow_admin_api=False) req = webob.Request.blank('/v1.1/servers?name=whee.*') res = req.get_response(fakes.wsgi_app()) @@ -1219,8 +1223,7 @@ class ServersTest(test.TestCase): """Test getting servers by instance_name with admin_api enabled but non-admin context """ - FLAGS.allow_admin_api = True - + self.flags(allow_admin_api=True) context = nova.context.RequestContext('testuser', 'testproject', is_admin=False) req = webob.Request.blank('/v1.1/servers?instance_name=whee.*') @@ -1234,8 +1237,6 @@ class ServersTest(test.TestCase): """Test getting servers by instance_name with admin_api enabled and admin context """ - FLAGS.allow_admin_api = True - def fake_get_all(compute_self, context, search_opts=None): self.assertNotEqual(search_opts, None) self.assertTrue('instance_name' in search_opts) @@ -1243,6 +1244,7 @@ class ServersTest(test.TestCase): return [stub_instance(100)] self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + self.flags(allow_admin_api=True) req = webob.Request.blank('/v1.1/servers?instance_name=whee.*') # Request admin context @@ -1878,6 +1880,7 @@ class ServersTest(test.TestCase): def test_get_all_server_details_v1_0(self): req = webob.Request.blank('/v1.0/servers/detail') res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 200) res_dict = json.loads(res.body) for i, s in enumerate(res_dict['servers']): @@ -1933,7 +1936,7 @@ class ServersTest(test.TestCase): return [stub_instance(i, 'fake', 'fake', None, None, i % 2) for i in xrange(5)] - self.stubs.Set(nova.db.api, 'instance_get_all_by_project', + self.stubs.Set(nova.db.api, 'instance_get_all_by_filters', return_servers_with_host) req = webob.Request.blank('/v1.0/servers/detail') -- cgit From e36232aed703eca43c6eb6df02a5c2aa0a1ac649 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 14:40:06 -0700 Subject: fix OS API tests --- nova/api/openstack/servers.py | 5 ++ nova/tests/api/openstack/fakes.py | 8 +- nova/tests/api/openstack/test_servers.py | 124 ++++++++++++++++++++++++------- 3 files changed, 109 insertions(+), 28 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index c842fcc01..e10e5bc86 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -59,9 +59,14 @@ class Controller(object): # Otherwise, strip out all unknown options unknown_options = [opt for opt in search_options if opt not in self.servers_search_options] + unk_opt_str = ", ".join(unknown_options) + log_msg = _("Stripping out options '%(unk_opt_str)s' from servers " + "query") % locals() + LOG.debug(log_msg) for opt in unknown_options: search_options.pop(opt, None) + def index(self, req): """ Returns a list of server names and ids for a given user """ try: diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py index a67a28a4e..d11fbf788 100644 --- a/nova/tests/api/openstack/fakes.py +++ b/nova/tests/api/openstack/fakes.py @@ -71,14 +71,18 @@ def fake_wsgi(self, req): return self.application -def wsgi_app(inner_app10=None, inner_app11=None, fake_auth=True): +def wsgi_app(inner_app10=None, inner_app11=None, fake_auth=True, + fake_auth_context=None): if not inner_app10: inner_app10 = openstack.APIRouterV10() if not inner_app11: inner_app11 = openstack.APIRouterV11() if fake_auth: - ctxt = context.RequestContext('fake', 'fake') + if fake_auth_context is not None: + ctxt = fake_auth_context + else: + ctxt = context.RequestContext('fake', 'fake') api10 = openstack.FaultWrapper(wsgi.InjectContext(ctxt, limits.RateLimitingMiddleware(inner_app10))) api11 = openstack.FaultWrapper(wsgi.InjectContext(ctxt, diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 2ae45b791..cc2afa57c 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -240,7 +240,8 @@ class ServersTest(test.TestCase): fakes.stub_out_key_pair_funcs(self.stubs) fakes.stub_out_image_service(self.stubs) self.stubs.Set(utils, 'gen_uuid', fake_gen_uuid) - self.stubs.Set(nova.db.api, 'instance_get_all', return_servers) + self.stubs.Set(nova.db.api, 'instance_get_all_by_filters', + return_servers) self.stubs.Set(nova.db.api, 'instance_get', return_server_by_id) self.stubs.Set(nova.db, 'instance_get_by_uuid', return_server_by_uuid) @@ -1165,7 +1166,7 @@ class ServersTest(test.TestCase): self.assertNotEqual(search_opts, None) self.assertTrue('flavor' in search_opts) # flavor is an integer ID - self.assertEqual(search_opts['flavor'], 12345) + self.assertEqual(search_opts['flavor'], '12345') return [stub_instance(100)] self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) @@ -1200,6 +1201,18 @@ class ServersTest(test.TestCase): self.assertEqual(len(servers), 1) self.assertEqual(servers[0]['id'], 100) + def test_get_servers_invalid_status_v1_1(self): + """Test getting servers by invalid status""" + + self.flags(allow_admin_api=False) + + req = webob.Request.blank('/v1.1/servers?status=running') + res = req.get_response(fakes.wsgi_app()) + # The following assert will fail if either of the asserts in + # fake_get_all() fail + self.assertEqual(res.status_int, 400) + self.assertTrue(res.body.find('Invalid server status') > -1) + def test_get_servers_allows_name_v1_1(self): def fake_get_all(compute_self, context, search_opts=None): self.assertNotEqual(search_opts, None) @@ -1219,39 +1232,100 @@ class ServersTest(test.TestCase): self.assertEqual(len(servers), 1) self.assertEqual(servers[0]['id'], 100) - def test_get_servers_allows_instance_name1_v1_1(self): - """Test getting servers by instance_name with admin_api - enabled but non-admin context + 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 + unknown options are stripped before they get to + compute_api.get_all() + """ + + self.flags(allow_admin_api=False) + + def fake_get_all(compute_self, context, search_opts=None): + self.assertNotEqual(search_opts, None) + # Allowed by user + self.assertTrue('name' in search_opts) + self.assertTrue('status' in search_opts) + # Allowed only by admins with admin API on + self.assertFalse('ip' in search_opts) + self.assertFalse('unknown_option' in search_opts) + return [stub_instance(100)] + + self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + + query_str = "name=foo&ip=10.*&status=active&unknown_option=meow" + req = webob.Request.blank('/v1.1/servers?%s' % query_str) + # Request admin context + context = nova.context.RequestContext('testuser', 'testproject', + is_admin=True) + res = req.get_response(fakes.wsgi_app(fake_auth_context=context)) + # The following assert will fail if either of the asserts in + # fake_get_all() fail + 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_unknown_or_admin_options2(self): + """Test getting servers by admin-only or unknown options. + This tests when admin_api is on, but context is a user. + Make sure the admin and unknown options are stripped before + they get to compute_api.get_all() """ + self.flags(allow_admin_api=True) + + def fake_get_all(compute_self, context, search_opts=None): + self.assertNotEqual(search_opts, None) + # Allowed by user + self.assertTrue('name' in search_opts) + self.assertTrue('status' in search_opts) + # Allowed only by admins with admin API on + self.assertFalse('ip' in search_opts) + self.assertFalse('unknown_option' in search_opts) + return [stub_instance(100)] + + self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) + + query_str = "name=foo&ip=10.*&status=active&unknown_option=meow" + req = webob.Request.blank('/v1.1/servers?%s' % query_str) + # Request admin context context = nova.context.RequestContext('testuser', 'testproject', is_admin=False) - req = webob.Request.blank('/v1.1/servers?instance_name=whee.*') - req.environ["nova.context"] = context - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 403) - self.assertTrue(res.body.find( - "User does not have admin privileges") > -1) + res = req.get_response(fakes.wsgi_app(fake_auth_context=context)) + # The following assert will fail if either of the asserts in + # fake_get_all() fail + 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_instance_name2_v1_1(self): - """Test getting servers by instance_name with admin_api - enabled and admin context + def test_get_servers_unknown_or_admin_options3(self): + """Test getting servers by admin-only or unknown options. + This tests when admin_api is on and context is admin. + All options should be passed through to compute_api.get_all() """ + + self.flags(allow_admin_api=True) + def fake_get_all(compute_self, context, search_opts=None): self.assertNotEqual(search_opts, None) - self.assertTrue('instance_name' in search_opts) - self.assertEqual(search_opts['instance_name'], 'whee.*') + # Allowed by user + self.assertTrue('name' in search_opts) + self.assertTrue('status' in search_opts) + # Allowed only by admins with admin API on + self.assertTrue('ip' in search_opts) + self.assertTrue('unknown_option' in search_opts) return [stub_instance(100)] self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) - self.flags(allow_admin_api=True) - req = webob.Request.blank('/v1.1/servers?instance_name=whee.*') + query_str = "name=foo&ip=10.*&status=active&unknown_option=meow" + req = webob.Request.blank('/v1.1/servers?%s' % query_str) # Request admin context context = nova.context.RequestContext('testuser', 'testproject', is_admin=True) - req.environ["nova.context"] = context - res = req.get_response(fakes.wsgi_app()) + res = req.get_response(fakes.wsgi_app(fake_auth_context=context)) # The following assert will fail if either of the asserts in # fake_get_all() fail self.assertEqual(res.status_int, 200) @@ -1259,7 +1333,7 @@ class ServersTest(test.TestCase): self.assertEqual(len(servers), 1) self.assertEqual(servers[0]['id'], 100) - def test_get_servers_allows_ip_v1_1(self): + def test_get_servers_admin_allows_ip_v1_1(self): """Test getting servers by ip with admin_api enabled and admin context """ @@ -1277,8 +1351,7 @@ class ServersTest(test.TestCase): # Request admin context context = nova.context.RequestContext('testuser', 'testproject', is_admin=True) - req.environ["nova.context"] = context - res = req.get_response(fakes.wsgi_app()) + res = req.get_response(fakes.wsgi_app(fake_auth_context=context)) # The following assert will fail if either of the asserts in # fake_get_all() fail self.assertEqual(res.status_int, 200) @@ -1286,7 +1359,7 @@ class ServersTest(test.TestCase): self.assertEqual(len(servers), 1) self.assertEqual(servers[0]['id'], 100) - def test_get_servers_allows_ip6_v1_1(self): + def test_get_servers_admin_allows_ip6_v1_1(self): """Test getting servers by ip6 with admin_api enabled and admin context """ @@ -1304,8 +1377,7 @@ class ServersTest(test.TestCase): # Request admin context context = nova.context.RequestContext('testuser', 'testproject', is_admin=True) - req.environ["nova.context"] = context - res = req.get_response(fakes.wsgi_app()) + res = req.get_response(fakes.wsgi_app(fake_auth_context=context)) # The following assert will fail if either of the asserts in # fake_get_all() fail self.assertEqual(res.status_int, 200) -- cgit From 2b45204e593f9330c8b961cfae3ad5af0bd36642 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 14:47:05 -0700 Subject: doc string fix --- nova/api/openstack/servers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index e10e5bc86..b89b24047 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -145,7 +145,8 @@ class Controller(object): def _servers_from_request(self, req, is_detail): """Returns a list of servers based on the request. - Checks for search options and permissions on the options. + Checks for search options and strips out options that should + not be available to non-admins. """ search_opts = {} -- cgit From 04a2a64d42e6acf0addd8918acd3139dc4aff7c7 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 15:04:36 -0700 Subject: resolved conflict incorrectly from trunk merge --- nova/tests/api/openstack/test_servers.py | 193 ------------------------------- 1 file changed, 193 deletions(-) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 37546dab4..41bbb91f5 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -2142,199 +2142,6 @@ class ServersTest(test.TestCase): self.assertEqual(res.status_int, 204) self.assertEqual(self.server_delete_called, True) - def test_resize_server(self): - req = self.webreq('/1/action', 'POST', dict(resize=dict(flavorId=3))) - - self.resize_called = False - - def resize_mock(*args): - self.resize_called = True - - self.stubs.Set(nova.compute.api.API, 'resize', resize_mock) - - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 202) - self.assertEqual(self.resize_called, True) - - def test_resize_server_v1_1(self): - req = webob.Request.blank('/v1.1/servers/1/action') - req.content_type = 'application/json' - req.method = 'POST' - body_dict = { - "resize": { - "flavorRef": 3, - }, - } - req.body = json.dumps(body_dict) - - self.resize_called = False - - def resize_mock(*args): - self.resize_called = True - - self.stubs.Set(nova.compute.api.API, 'resize', resize_mock) - - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 202) - self.assertEqual(self.resize_called, True) - - def test_resize_bad_flavor_data(self): - req = self.webreq('/1/action', 'POST', {"resize": "bad_data"}) - - self.resize_called = False - - def resize_mock(*args): - self.resize_called = True - - self.stubs.Set(nova.compute.api.API, 'resize', resize_mock) - - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 400) - self.assertEqual(self.resize_called, False) - - def test_resize_invalid_flavorid(self): - req = self.webreq('/1/action', 'POST', {"resize": {"flavorId": 300}}) - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 400) - - def test_resize_nonint_flavorid(self): - req = self.webreq('/1/action', 'POST', {"resize": {"flavorId": "a"}}) - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 400) - - def test_resize_invalid_flavorid_v1_1(self): - req = webob.Request.blank('/v1.1/servers/1/action') - req.content_type = 'application/json' - req.method = 'POST' - resize_body = { - "resize": { - "image": { - "id": 300, - }, - }, - } - req.body = json.dumps(resize_body) - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 400) - - def test_resize_nonint_flavorid_v1_1(self): - req = webob.Request.blank('/v1.1/servers/1/action') - req.content_type = 'application/json' - req.method = 'POST' - resize_body = { - "resize": { - "image": { - "id": "a", - }, - }, - } - req.body = json.dumps(resize_body) - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 400) - - def test_resize_raises_fails(self): - req = self.webreq('/1/action', 'POST', dict(resize=dict(flavorId=3))) - - def resize_mock(*args): - raise Exception("An error occurred.") - - self.stubs.Set(nova.compute.api.API, 'resize', resize_mock) - - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 500) - - def test_resized_server_has_correct_status(self): - req = self.webreq('/1', 'GET') - - def fake_migration_get(*args): - return {} - - self.stubs.Set(nova.db, 'migration_get_by_instance_and_status', - fake_migration_get) - res = req.get_response(fakes.wsgi_app()) - body = json.loads(res.body) - self.assertEqual(body['server']['status'], 'RESIZE-CONFIRM') - - def test_confirm_resize_server(self): - req = self.webreq('/1/action', 'POST', dict(confirmResize=None)) - - self.resize_called = False - - def confirm_resize_mock(*args): - self.resize_called = True - - self.stubs.Set(nova.compute.api.API, 'confirm_resize', - confirm_resize_mock) - - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 204) - self.assertEqual(self.resize_called, True) - - def test_confirm_resize_server_fails(self): - req = self.webreq('/1/action', 'POST', dict(confirmResize=None)) - - def confirm_resize_mock(*args): - raise Exception("An error occurred.") - - self.stubs.Set(nova.compute.api.API, 'confirm_resize', - confirm_resize_mock) - - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 400) - - def test_revert_resize_server(self): - req = self.webreq('/1/action', 'POST', dict(revertResize=None)) - - self.resize_called = False - - def revert_resize_mock(*args): - self.resize_called = True - - self.stubs.Set(nova.compute.api.API, 'revert_resize', - revert_resize_mock) - - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 202) - self.assertEqual(self.resize_called, True) - - def test_revert_resize_server_fails(self): - req = self.webreq('/1/action', 'POST', dict(revertResize=None)) - - def revert_resize_mock(*args): - raise Exception("An error occurred.") - - self.stubs.Set(nova.compute.api.API, 'revert_resize', - revert_resize_mock) - - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 400) - - def test_migrate_server(self): - """This is basically the same as resize, only we provide the `migrate` - attribute in the body's dict. - """ - req = self.webreq('/1/migrate', 'POST') - - FLAGS.allow_admin_api = True - self.resize_called = False - - def resize_mock(*args): - self.resize_called = True - - self.stubs.Set(nova.compute.api.API, 'resize', resize_mock) - - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 202) - self.assertEqual(self.resize_called, True) - - def test_migrate_server_no_admin_api_fails(self): - req = self.webreq('/1/migrate', 'POST') - - FLAGS.allow_admin_api = False - - res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 404) - def test_shutdown_status(self): new_server = return_server_with_power_state(power_state.SHUTDOWN) self.stubs.Set(nova.db.api, 'instance_get', new_server) -- cgit From 6e791e8b773565b62c4b8ba35cec455cb8c13ac8 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 16:30:55 -0700 Subject: test fixes.. one more to go --- nova/compute/api.py | 38 ++++++++++++++++++++++++++++---------- nova/db/sqlalchemy/api.py | 8 +------- nova/tests/test_compute.py | 35 ++++++++++++++++++++++++++--------- 3 files changed, 55 insertions(+), 26 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 02e9f3e06..382f3c541 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -684,25 +684,43 @@ class API(base.Base): # Fixups for the DB call filters = search_opts.copy() + recurse_zones = filters.pop('recurse_zones', False) if 'image' in filters: filters['image_ref'] = filters['image'] del filters['image'] + invalid_flavor = False if 'flavor' in filters: - flavor_id = int(filters['flavor']) - try: - instance_type = self.db.instance_type_get_by_flavor_id( - context, flavor_id) - except exception.FlavorNotFound: - pass - else: - filters['instance_type_id'] = instance_type['id'] + instance_type = self.db.instance_type_get_by_flavor_id( + context, filters['flavor']) + filters['instance_type_id'] = instance_type['id'] del filters['flavor'] + # 'name' means Instance.display_name + # 'instance_name' means Instance.name + if 'name' in filters: + filters['display_name'] = filters['name'] + del filters['name'] + if 'instance_name' in filters: + filters['name'] = filters['instance_name'] + del filters['instance_name'] - recurse_zones = filters.pop('recurse_zones', False) if 'reservation_id' in filters: recurse_zones = True - instances = self.db.instance_get_all_by_filters(context, filters) + if 'fixed_ip' in search_opts: + # special cased for ec2. we end up ignoring all other + # search options. + try: + instance = self.db.instance_get_by_fixed_ip(context, + search_opts['fixed_ip']) + except exception.FloatingIpNotFound, e: + if not recurse_zones: + raise + if instance: + return [instance] + instances = [] + # fall through + else: + instances = self.db.instance_get_all_by_filters(context, filters) if not recurse_zones: return instances diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index f8920e62c..4d724dbdb 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1174,11 +1174,6 @@ def instance_get_all_by_filters(context, filters): return True return False - def _regexp_filter_by_display_name(instance, filter_re): - if filter_re.match(instance.display_name): - return True - return False - def _regexp_filter_by_column(instance, filter_name, filter_re): try: v = getattr(instance, filter_name) @@ -1243,8 +1238,7 @@ def instance_get_all_by_filters(context, filters): # For filters not in the list, we'll attempt to use the filter_name # as a column name in Instance.. regexp_filter_funcs = {'ip6': _regexp_filter_by_ipv6, - 'ip': _regexp_filter_by_ip, - 'name': _regexp_filter_by_display_name} + 'ip': _regexp_filter_by_ip} for filter_name in filters.iterkeys(): filter_func = regexp_filter_funcs.get(filter_name, None) diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index 6732df154..0957981ed 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -980,7 +980,7 @@ class ComputeTestCase(test.TestCase): db.instance_destroy(c, instance_id1) db.instance_destroy(c, instance_id2) - def test_get_all_by_ip_regex(self): + def test_get_all_by_ip_regexp(self): """Test searching by Floating and Fixed IP""" c = context.get_admin_context() instance_id1 = self._create_instance({'server_name': 'woot'}) @@ -991,20 +991,34 @@ class ComputeTestCase(test.TestCase): 'server_name': 'not-woot', 'id': 30}) + vif_ref1 = db.virtual_interface_create(c, + {'instance_id': instance_id1, + 'network_id': 1}) + vif_ref2 = db.virtual_interface_create(c, + {'instance_id': instance_id2, + 'network_id': 2}) + vif_ref3 = db.virtual_interface_create(c, + {'instance_id': instance_id3, + 'network_id': 3}) + db.fixed_ip_create(c, {'address': '1.1.1.1', - 'instance_id': instance_id1}) + 'instance_id': instance_id1, + 'virtual_interface_id': vif_ref1['id']}) db.fixed_ip_create(c, {'address': '1.1.2.1', - 'instance_id': instance_id2}) + 'instance_id': instance_id2, + 'virtual_interface_id': vif_ref2['id']}) fix_addr = db.fixed_ip_create(c, {'address': '1.1.3.1', - 'instance_id': instance_id3}) + 'instance_id': instance_id3, + 'virtual_interface_id': vif_ref3['id']}) fix_ref = db.fixed_ip_get_by_address(c, fix_addr) flo_ref = db.floating_ip_create(c, {'address': '10.0.0.2', 'fixed_ip_id': fix_ref['id']}) + # ends up matching 2nd octet here.. so all 3 match instances = self.compute_api.get_all(c, search_opts={'ip': '.*\.1'}) self.assertEqual(len(instances), 3) @@ -1029,12 +1043,15 @@ class ComputeTestCase(test.TestCase): self.assertEqual(len(instances), 1) self.assertEqual(instances[0].id, instance_id3) + db.virtual_interface_delete(c, vif_ref1['id']) + db.virtual_interface_delete(c, vif_ref2['id']) + db.virtual_interface_delete(c, vif_ref3['id']) + db.floating_ip_destroy(c, '10.0.0.2') db.instance_destroy(c, instance_id1) db.instance_destroy(c, instance_id2) db.instance_destroy(c, instance_id3) - db.floating_ip_destroy(c, '10.0.0.2') - def test_get_all_by_ipv6_regex(self): + def test_get_all_by_ipv6_regexp(self): """Test searching by IPv6 address""" def fake_ipv6_get_by_instance_ref(context, instance): if instance.id == 1: @@ -1144,9 +1161,9 @@ class ComputeTestCase(test.TestCase): search_opts={'flavor': 5}) self.assertEqual(len(instances), 0) - instances = self.compute_api.get_all(c, - search_opts={'flavor': 99}) - self.assertEqual(len(instances), 0) + self.assertRaises(exception.FlavorNotFound, + self.compute_api.get_all, + c, search_opts={'flavor': 99}) instances = self.compute_api.get_all(c, search_opts={'flavor': 3}) -- cgit From d722d6f635c99a758910f24e7681753599894e70 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 17:09:36 -0700 Subject: fix ipv6 search test and add test for multiple options at once --- nova/tests/test_compute.py | 142 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 116 insertions(+), 26 deletions(-) diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index 0957981ed..7792f5909 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -983,23 +983,26 @@ class ComputeTestCase(test.TestCase): def test_get_all_by_ip_regexp(self): """Test searching by Floating and Fixed IP""" c = context.get_admin_context() - instance_id1 = self._create_instance({'server_name': 'woot'}) + instance_id1 = self._create_instance({'display_name': 'woot'}) instance_id2 = self._create_instance({ - 'server_name': 'woo', + 'display_name': 'woo', 'id': 20}) instance_id3 = self._create_instance({ - 'server_name': 'not-woot', + 'display_name': 'not-woot', 'id': 30}) vif_ref1 = db.virtual_interface_create(c, - {'instance_id': instance_id1, + {'address': '12:34:56:78:90:12', + 'instance_id': instance_id1, 'network_id': 1}) vif_ref2 = db.virtual_interface_create(c, - {'instance_id': instance_id2, - 'network_id': 2}) + {'address': '90:12:34:56:78:90', + 'instance_id': instance_id2, + 'network_id': 1}) vif_ref3 = db.virtual_interface_create(c, - {'instance_id': instance_id3, - 'network_id': 3}) + {'address': '34:56:78:90:12:34', + 'instance_id': instance_id3, + 'network_id': 1}) db.fixed_ip_create(c, {'address': '1.1.1.1', @@ -1053,35 +1056,41 @@ class ComputeTestCase(test.TestCase): def test_get_all_by_ipv6_regexp(self): """Test searching by IPv6 address""" - def fake_ipv6_get_by_instance_ref(context, instance): - if instance.id == 1: - return ['ffff:ffff::1'] - if instance.id == 20: - return ['dddd:dddd::1'] - if instance.id == 30: - return ['cccc:cccc::1', 'eeee:eeee::1', 'dddd:dddd::1'] - - self.stubs.Set(sqlalchemy_api, '_ipv6_get_by_instance_ref', - fake_ipv6_get_by_instance_ref) c = context.get_admin_context() - instance_id1 = self._create_instance({'server_name': 'woot'}) + instance_id1 = self._create_instance({'display_name': 'woot'}) instance_id2 = self._create_instance({ - 'server_name': 'woo', + 'display_name': 'woo', 'id': 20}) instance_id3 = self._create_instance({ - 'server_name': 'not-woot', + 'display_name': 'not-woot', 'id': 30}) + vif_ref1 = db.virtual_interface_create(c, + {'address': '12:34:56:78:90:12', + 'instance_id': instance_id1, + 'network_id': 1}) + vif_ref2 = db.virtual_interface_create(c, + {'address': '90:12:34:56:78:90', + 'instance_id': instance_id2, + 'network_id': 1}) + vif_ref3 = db.virtual_interface_create(c, + {'address': '34:56:78:90:12:34', + 'instance_id': instance_id3, + 'network_id': 1}) + + # This will create IPv6 addresses of: + # 1: fd00::1034:56ff:fe78:9012 + # 20: fd00::9212:34ff:fe56:7890 + # 30: fd00::3656:78ff:fe90:1234 + instances = self.compute_api.get_all(c, - search_opts={'ip6': 'ff.*'}) + search_opts={'ip6': '.*1034.*'}) self.assertEqual(len(instances), 1) self.assertEqual(instances[0].id, instance_id1) - instance_ids = [instance.id for instance in instances] - self.assertTrue(instance_id1 in instance_ids) instances = self.compute_api.get_all(c, - search_opts={'ip6': '.*::1'}) + search_opts={'ip6': '^fd00.*'}) self.assertEqual(len(instances), 3) instance_ids = [instance.id for instance in instances] self.assertTrue(instance_id1 in instance_ids) @@ -1089,12 +1098,93 @@ class ComputeTestCase(test.TestCase): self.assertTrue(instance_id3 in instance_ids) instances = self.compute_api.get_all(c, - search_opts={'ip6': '.*dd:.*'}) + search_opts={'ip6': '^.*12.*34.*'}) self.assertEqual(len(instances), 2) instance_ids = [instance.id for instance in instances] self.assertTrue(instance_id2 in instance_ids) self.assertTrue(instance_id3 in instance_ids) + db.virtual_interface_delete(c, vif_ref1['id']) + db.virtual_interface_delete(c, vif_ref2['id']) + db.virtual_interface_delete(c, vif_ref3['id']) + db.instance_destroy(c, instance_id1) + db.instance_destroy(c, instance_id2) + db.instance_destroy(c, instance_id3) + + def test_get_all_by_multiple_options_at_once(self): + """Test searching by multiple options at once""" + c = context.get_admin_context() + instance_id1 = self._create_instance({'display_name': 'woot'}) + instance_id2 = self._create_instance({ + 'display_name': 'woo', + 'id': 20}) + instance_id3 = self._create_instance({ + 'display_name': 'not-woot', + 'id': 30}) + + vif_ref1 = db.virtual_interface_create(c, + {'address': '12:34:56:78:90:12', + 'instance_id': instance_id1, + 'network_id': 1}) + vif_ref2 = db.virtual_interface_create(c, + {'address': '90:12:34:56:78:90', + 'instance_id': instance_id2, + 'network_id': 1}) + vif_ref3 = db.virtual_interface_create(c, + {'address': '34:56:78:90:12:34', + 'instance_id': instance_id3, + 'network_id': 1}) + + db.fixed_ip_create(c, + {'address': '1.1.1.1', + 'instance_id': instance_id1, + 'virtual_interface_id': vif_ref1['id']}) + db.fixed_ip_create(c, + {'address': '1.1.2.1', + 'instance_id': instance_id2, + 'virtual_interface_id': vif_ref2['id']}) + fix_addr = db.fixed_ip_create(c, + {'address': '1.1.3.1', + 'instance_id': instance_id3, + 'virtual_interface_id': vif_ref3['id']}) + fix_ref = db.fixed_ip_get_by_address(c, fix_addr) + flo_ref = db.floating_ip_create(c, + {'address': '10.0.0.2', + 'fixed_ip_id': fix_ref['id']}) + + # ip ends up matching 2nd octet here.. so all 3 match ip + # but 'name' only matches one + instances = self.compute_api.get_all(c, + search_opts={'ip': '.*\.1', 'name': 'not.*'}) + self.assertEqual(len(instances), 1) + self.assertEqual(instances[0].id, instance_id3) + + # ip ends up matching any ip with a '2' in it.. so instance + # 2 and 3.. but name should only match #2 + # but 'name' only matches one + instances = self.compute_api.get_all(c, + search_opts={'ip': '.*2', 'name': '^woo.*'}) + self.assertEqual(len(instances), 1) + self.assertEqual(instances[0].id, instance_id2) + + # same as above but no match on name (name matches instance_id1 + # but the ip query doesn't + instances = self.compute_api.get_all(c, + search_opts={'ip': '.*2.*', 'name': '^woot.*'}) + self.assertEqual(len(instances), 0) + + # ip matches all 3... ipv6 matches #2+#3...name matches #3 + instances = self.compute_api.get_all(c, + search_opts={'ip': '.*\.1', + 'name': 'not.*', + 'ip6': '^.*12.*34.*'}) + self.assertEqual(len(instances), 1) + self.assertEqual(instances[0].id, instance_id3) + + db.virtual_interface_delete(c, vif_ref1['id']) + db.virtual_interface_delete(c, vif_ref2['id']) + db.virtual_interface_delete(c, vif_ref3['id']) + db.floating_ip_destroy(c, '10.0.0.2') db.instance_destroy(c, instance_id1) db.instance_destroy(c, instance_id2) db.instance_destroy(c, instance_id3) -- cgit From 64966cbd83cbde6a240dad4ac786fe7a6a116f2f Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 17:30:07 -0700 Subject: pep8 fixes --- nova/api/openstack/servers.py | 1 - nova/db/api.py | 1 + nova/db/sqlalchemy/api.py | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 63791bcd1..b54897b8a 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -66,7 +66,6 @@ class Controller(object): for opt in unknown_options: search_options.pop(opt, None) - def index(self, req): """ Returns a list of server names and ids for a given user """ try: diff --git a/nova/db/api.py b/nova/db/api.py index 23fac9921..4c8f25f5d 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -495,6 +495,7 @@ def instance_get_all_by_filters(context, filters): """Get all instances that match all filters.""" return IMPL.instance_get_all_by_filters(context, filters) + def instance_get_active_by_window(context, begin, end=None): """Get instances active during a certain time window.""" return IMPL.instance_get_active_by_window(context, begin, end) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 4d724dbdb..36fae1be1 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1205,7 +1205,7 @@ def instance_get_all_by_filters(context, filters): options(joinedload_all('fixed_ips.network')).\ options(joinedload('metadata')).\ options(joinedload('instance_type')) - + filters = filters.copy() if not context.is_admin: -- cgit From 8aeec07c2a5f8a5f1cfb049e20caa29295496606 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 17:36:39 -0700 Subject: add comment for servers_search_options list in the OS API Controllers. --- nova/api/openstack/servers.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index b54897b8a..ae0b103bd 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -46,6 +46,10 @@ FLAGS = flags.FLAGS class Controller(object): """ The Server API base controller class for the OpenStack API """ + # These are a list of possible query string paramters to the + # /servers query that a user should be able to do. Specify this + # in your subclasses. When admin api is off, unknown options will + # get filtered out without error. servers_search_options = [] def __init__(self): @@ -565,6 +569,9 @@ class Controller(object): class ControllerV10(Controller): """v1.0 OpenStack API controller""" + # These are a list of possible query string paramters to the + # /servers query that a user should be able to do. When admin api + # is off, unknown options will get filtered out without error. servers_search_options = ["reservation_id", "fixed_ip", "name", "recurse_zones"] @@ -633,6 +640,9 @@ class ControllerV10(Controller): class ControllerV11(Controller): """v1.1 OpenStack API controller""" + # These are a list of possible query string paramters to the + # /servers query that a user should be able to do. When admin api + # is off, unknown options will get filtered out without error. servers_search_options = ["reservation_id", "name", "recurse_zones", "status", "image", "flavor", "changes-since"] -- cgit From 4d0125b34a4796fd6d3312a4144a0834ba318469 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 17:50:59 -0700 Subject: convert filter value to a string just in case before running re.compile --- 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 36fae1be1..d6e7204b4 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1242,7 +1242,7 @@ def instance_get_all_by_filters(context, filters): for filter_name in filters.iterkeys(): filter_func = regexp_filter_funcs.get(filter_name, None) - filter_re = re.compile(filters[filter_name]) + filter_re = re.compile(str(filters[filter_name])) if filter_func: filter_l = lambda instance: filter_func(instance, filter_re) else: -- cgit From fd9a761f25c6095d1ea47e09cdac503683b44bfc Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Thu, 4 Aug 2011 17:59:22 -0700 Subject: pep8 fix --- nova/api/openstack/servers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index ae0b103bd..b3776ed44 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -46,7 +46,7 @@ FLAGS = flags.FLAGS class Controller(object): """ The Server API base controller class for the OpenStack API """ - # These are a list of possible query string paramters to the + # These are a list of possible query string paramters to the # /servers query that a user should be able to do. Specify this # in your subclasses. When admin api is off, unknown options will # get filtered out without error. @@ -569,7 +569,7 @@ class Controller(object): class ControllerV10(Controller): """v1.0 OpenStack API controller""" - # These are a list of possible query string paramters to the + # These are a list of possible query string paramters to the # /servers query that a user should be able to do. When admin api # is off, unknown options will get filtered out without error. servers_search_options = ["reservation_id", "fixed_ip", @@ -640,7 +640,7 @@ class ControllerV10(Controller): class ControllerV11(Controller): """v1.1 OpenStack API controller""" - # These are a list of possible query string paramters to the + # These are a list of possible query string paramters to the # /servers query that a user should be able to do. When admin api # is off, unknown options will get filtered out without error. servers_search_options = ["reservation_id", "name", "recurse_zones", -- cgit From b2fe710c59ba266b9afd67db1cae60a6db5c71e3 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Fri, 5 Aug 2011 15:06:07 -0700 Subject: rename _check_servers_options, add some comments and small cleanup in the db get_by_filters call --- nova/api/openstack/servers.py | 4 ++-- nova/db/sqlalchemy/api.py | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index b3776ed44..9a3872113 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -56,7 +56,7 @@ class Controller(object): self.compute_api = compute.API() self.helper = helper.CreateInstanceHelper(self) - def _check_servers_options(self, context, search_options): + def _remove_invalid_options(self, context, search_options): if FLAGS.allow_admin_api and context.is_admin: # Allow all options return @@ -156,7 +156,7 @@ class Controller(object): search_opts.update(req.str_GET) context = req.environ['nova.context'] - self._check_servers_options(context, search_opts) + self._remove_invalid_options(context, search_opts) # Convert recurse_zones into a boolean search_opts['recurse_zones'] = utils.bool_from_str( diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index d6e7204b4..65a1c19a1 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1206,6 +1206,8 @@ def instance_get_all_by_filters(context, filters): options(joinedload('metadata')).\ options(joinedload('instance_type')) + # 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 not context.is_admin: @@ -1224,10 +1226,10 @@ def instance_get_all_by_filters(context, filters): if key in exact_match_filter_names] for filter_name in query_filters: + # Do the matching and remove the filter from the dictionary + # so we don't try it again below.. query_prefix = _exact_match_filter(query_prefix, filter_name, - filters[filter_name]) - # Remove this from filters, so it doesn't get tried below - del filters[filter_name] + filters.pop(filter_name)) instances = query_prefix.all() -- cgit From ca152762aa73a93583be2ada237cf8bbbcc99220 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 6 Jul 2011 05:41:47 -0700 Subject: clean up OS API servers getting --- nova/api/openstack/servers.py | 94 +++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 57 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 9a3872113..ce04a1eab 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -46,34 +46,14 @@ FLAGS = flags.FLAGS class Controller(object): """ The Server API base controller class for the OpenStack API """ - # These are a list of possible query string paramters to the - # /servers query that a user should be able to do. Specify this - # in your subclasses. When admin api is off, unknown options will - # get filtered out without error. - servers_search_options = [] - def __init__(self): self.compute_api = compute.API() self.helper = helper.CreateInstanceHelper(self) - def _remove_invalid_options(self, context, search_options): - if FLAGS.allow_admin_api and context.is_admin: - # Allow all options - return - # Otherwise, strip out all unknown options - unknown_options = [opt for opt in search_options - if opt not in self.servers_search_options] - unk_opt_str = ", ".join(unknown_options) - log_msg = _("Stripping out options '%(unk_opt_str)s' from servers " - "query") % locals() - LOG.debug(log_msg) - for opt in unknown_options: - search_options.pop(opt, None) - def index(self, req): """ Returns a list of server names and ids for a given user """ try: - servers = self._servers_from_request(req, is_detail=False) + servers = self._get_servers(req, is_detail=False) except exception.Invalid as err: return exc.HTTPBadRequest(explanation=str(err)) except exception.NotFound: @@ -83,7 +63,7 @@ class Controller(object): def detail(self, req): """ Returns a list of server details for a given user """ try: - servers = self._servers_from_request(req, is_detail=True) + servers = self._get_servers(req, is_detail=True) except exception.Invalid as err: return exc.HTTPBadRequest(explanation=str(err)) except exception.NotFound as err: @@ -99,13 +79,21 @@ class Controller(object): def _action_rebuild(self, info, request, instance_id): raise NotImplementedError() - def _servers_search(self, context, req, is_detail, search_opts=None): + def _get_servers(self, req, is_detail): """Returns a list of servers, taking into account any search options specified. """ - if search_opts is None: - search_opts = {} + search_opts = {} + search_opts.update(req.str_GET) + + context = req.environ['nova.context'] + remove_invalid_options(context, search_opts, + self._get_server_search_options()) + + # Convert recurse_zones into a boolean + search_opts['recurse_zones'] = utils.bool_from_str( + search_opts.get('recurse_zones', False)) # If search by 'status', we need to convert it to 'state' # If the status is unknown, bail. @@ -145,26 +133,6 @@ class Controller(object): for inst in limited_list] return dict(servers=servers) - def _servers_from_request(self, req, is_detail): - """Returns a list of servers based on the request. - - Checks for search options and strips out options that should - not be available to non-admins. - """ - - search_opts = {} - search_opts.update(req.str_GET) - - context = req.environ['nova.context'] - self._remove_invalid_options(context, search_opts) - - # Convert recurse_zones into a boolean - search_opts['recurse_zones'] = utils.bool_from_str( - search_opts.get('recurse_zones', False)) - - return self._servers_search(context, req, is_detail, - search_opts=search_opts) - @scheduler_api.redirect_handler def show(self, req, id): """ Returns server details by server id """ @@ -569,12 +537,6 @@ class Controller(object): class ControllerV10(Controller): """v1.0 OpenStack API controller""" - # These are a list of possible query string paramters to the - # /servers query that a user should be able to do. When admin api - # is off, unknown options will get filtered out without error. - servers_search_options = ["reservation_id", "fixed_ip", - "name", "recurse_zones"] - @scheduler_api.redirect_handler def delete(self, req, id): """ Destroys a server """ @@ -636,16 +598,14 @@ class ControllerV10(Controller): """ Determine the admin password for a server on creation """ return self.helper._get_server_admin_password_old_style(server) + def _get_server_search_options(self): + """Return server search options allowed by non-admin""" + return 'reservation_id', 'fixed_ip', 'name', 'recurse_zones' + class ControllerV11(Controller): """v1.1 OpenStack API controller""" - # These are a list of possible query string paramters to the - # /servers query that a user should be able to do. When admin api - # is off, unknown options will get filtered out without error. - servers_search_options = ["reservation_id", "name", "recurse_zones", - "status", "image", "flavor", "changes-since"] - @scheduler_api.redirect_handler def delete(self, req, id): """ Destroys a server """ @@ -812,6 +772,11 @@ class ControllerV11(Controller): """ Determine the admin password for a server on creation """ return self.helper._get_server_admin_password_new_style(server) + def _get_server_search_options(self): + """Return server search options allowed by non-admin""" + return ('reservation_id', 'name', 'recurse_zones', + 'status', 'image', 'flavor', 'changes-since') + class HeadersSerializer(wsgi.ResponseHeadersSerializer): @@ -982,3 +947,18 @@ def create_resource(version='1.0'): deserializer = wsgi.RequestDeserializer(body_deserializers) return wsgi.Resource(controller, deserializer, serializer) + + +def remove_invalid_options(context, search_options, allowed_search_options): + """Remove search options that are not valid for non-admin API/context""" + if FLAGS.allow_admin_api and context.is_admin: + # Allow all options + return + # Otherwise, strip out all unknown options + unknown_options = [opt for opt in search_options + if opt not in allowed_search_options] + unk_opt_str = ", ".join(unknown_options) + log_msg = _("Removing options '%(unk_opt_str)s' from query") % locals() + LOG.debug(log_msg) + for opt in unknown_options: + search_options.pop(opt, None) -- cgit From 1c90eb34085dbb69f37e2f63dea7496afabb06b3 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 6 Jul 2011 06:20:38 -0700 Subject: clean up compute_api.get_all filter name remappings. ditch fixed_ip one-off code. fixed ec2 api call to this to compensate --- nova/api/ec2/cloud.py | 2 +- nova/compute/api.py | 75 +++++++++++++++++++++++----------------------- nova/tests/test_compute.py | 27 ++++++++++++----- 3 files changed, 57 insertions(+), 47 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 966c3a564..db49baffa 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -242,7 +242,7 @@ class CloudController(object): search_opts=search_opts) except exception.NotFound: instance_ref = None - if instance_ref is None: + if not instance_ref: return None # This ensures that all attributes of the instance diff --git a/nova/compute/api.py b/nova/compute/api.py index 382f3c541..7a3ff0c56 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -683,44 +683,49 @@ class API(base.Base): LOG.debug(_("Searching by: %s") % str(search_opts)) # Fixups for the DB call - filters = search_opts.copy() - recurse_zones = filters.pop('recurse_zones', False) - if 'image' in filters: - filters['image_ref'] = filters['image'] - del filters['image'] - invalid_flavor = False - if 'flavor' in filters: + filters = {} + + def _remap_flavor_filter(flavor_id): instance_type = self.db.instance_type_get_by_flavor_id( - context, filters['flavor']) + context, flavor_id) filters['instance_type_id'] = instance_type['id'] - del filters['flavor'] - # 'name' means Instance.display_name - # 'instance_name' means Instance.name - if 'name' in filters: - filters['display_name'] = filters['name'] - del filters['name'] - if 'instance_name' in filters: - filters['name'] = filters['instance_name'] - del filters['instance_name'] + def _remap_fixed_ip_filter(fixed_ip): + # Turn fixed_ip into a regexp match. Since '.' matches + # any character, we need to use regexp escaping for it. + filters['ip'] = '^%s$' % fixed_ip.replace('.', '\\.') + + # search_option to filter_name mapping. + filter_mapping = { + 'image': 'image_ref', + 'name': 'display_name', + 'instance_name': 'name', + 'recurse_zones': None, + 'flavor': _remap_flavor_filter, + 'fixed_ip': _remap_fixed_ip_filter} + + # copy from search_opts, doing various remappings as necessary + for opt, value in search_opts.iteritems(): + # Do remappings. + # Values not in the filter_mapping table are copied as-is. + # If remapping is None, option is not copied + # If the remapping is a string, it is the filter_name to use + try: + remap_object = filter_mapping[opt] + except KeyError: + filters[opt] = value + else: + if remap_object: + if isinstance(remap_object, basestring): + filters[remap_object] = value + else: + remap_object(value) + + recurse_zones = search_opts.get('recurse_zones', False) if 'reservation_id' in filters: recurse_zones = True - if 'fixed_ip' in search_opts: - # special cased for ec2. we end up ignoring all other - # search options. - try: - instance = self.db.instance_get_by_fixed_ip(context, - search_opts['fixed_ip']) - except exception.FloatingIpNotFound, e: - if not recurse_zones: - raise - if instance: - return [instance] - instances = [] - # fall through - else: - instances = self.db.instance_get_all_by_filters(context, filters) + instances = self.db.instance_get_all_by_filters(context, filters) if not recurse_zones: return instances @@ -743,12 +748,6 @@ class API(base.Base): server._info['_is_precooked'] = True instances.append(server._info) - # fixed_ip searching should return a FixedIpNotFound exception - # when an instance is not found... - fixed_ip = search_opts.get('fixed_ip', None) - if fixed_ip and not instances: - raise exception.FixedIpNotFoundForAddress(address=fixed_ip) - return instances def _cast_compute_message(self, method, context, instance_id, host=None, diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index 7792f5909..18ec08597 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -949,23 +949,32 @@ class ComputeTestCase(test.TestCase): instance_id2 = self._create_instance({'id': 20}) instance_id3 = self._create_instance({'id': 30}) + vif_ref1 = db.virtual_interface_create(c, + {'address': '12:34:56:78:90:12', + 'instance_id': instance_id1, + 'network_id': 1}) + vif_ref2 = db.virtual_interface_create(c, + {'address': '90:12:34:56:78:90', + 'instance_id': instance_id2, + 'network_id': 1}) + db.fixed_ip_create(c, {'address': '1.1.1.1', - 'instance_id': instance_id1}) + 'instance_id': instance_id1, + 'virtual_interface_id': vif_ref1['id']}) db.fixed_ip_create(c, {'address': '1.1.2.1', - 'instance_id': instance_id2}) + 'instance_id': instance_id2, + 'virtual_interface_id': vif_ref2['id']}) # regex not allowed - self.assertRaises(exception.NotFound, - self.compute_api.get_all, - c, + instances = self.compute_api.get_all(c, search_opts={'fixed_ip': '.*'}) + self.assertEqual(len(instances), 0) - self.assertRaises(exception.NotFound, - self.compute_api.get_all, - c, + instances = self.compute_api.get_all(c, search_opts={'fixed_ip': '1.1.3.1'}) + self.assertEqual(len(instances), 0) instances = self.compute_api.get_all(c, search_opts={'fixed_ip': '1.1.1.1'}) @@ -977,6 +986,8 @@ class ComputeTestCase(test.TestCase): self.assertEqual(len(instances), 1) self.assertEqual(instances[0].id, instance_id2) + db.virtual_interface_delete(c, vif_ref1['id']) + db.virtual_interface_delete(c, vif_ref2['id']) db.instance_destroy(c, instance_id1) db.instance_destroy(c, instance_id2) -- cgit From b19dbcf21865aa0d1b422aecdb7ff13571ecb4e8 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 6 Jul 2011 06:37:28 -0700 Subject: fix metadata test since fixed_ip searching now goes thru filters db api call instead of the get_by_fixed_ip call --- nova/tests/test_metadata.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nova/tests/test_metadata.py b/nova/tests/test_metadata.py index b9b14d1ea..d63394ad6 100644 --- a/nova/tests/test_metadata.py +++ b/nova/tests/test_metadata.py @@ -43,16 +43,20 @@ class MetadataTestCase(test.TestCase): 'reservation_id': 'r-xxxxxxxx', 'user_data': '', 'image_ref': 7, + 'fixed_ips': [], 'hostname': 'test'}) def instance_get(*args, **kwargs): return self.instance + def instance_get_list(*args, **kwargs): + return [self.instance] + def floating_get(*args, **kwargs): return '99.99.99.99' self.stubs.Set(api, 'instance_get', instance_get) - self.stubs.Set(api, 'instance_get_by_fixed_ip', instance_get) + self.stubs.Set(api, 'instance_get_all_by_filters', instance_get_list) self.stubs.Set(api, 'instance_get_floating_address', floating_get) self.app = metadatarequesthandler.MetadataRequestHandler() -- cgit From 65fcbc8cf51cc02071d1d9cd60cf0eb59c2bcce0 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 6 Jul 2011 06:44:50 -0700 Subject: merge code i'd split from instance_get_fixed_addresses_v6 that's no longer needed to be split --- nova/db/sqlalchemy/api.py | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 65a1c19a1..503c526f0 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1395,33 +1395,29 @@ def instance_get_fixed_addresses(context, instance_id): return [fixed_ip.address for fixed_ip in fixed_ips] -def _ipv6_get_by_instance_ref(context, instance_ref): - # assume instance has 1 mac for each network associated with it - # get networks associated with instance - network_refs = network_get_all_by_instance(context, instance_id) - # compile a list of cidr_v6 prefixes sorted by network id - prefixes = [ref.cidr_v6 for ref in - sorted(network_refs, key=lambda ref: ref.id)] - # get vifs associated with instance - vif_refs = virtual_interface_get_by_instance(context, instance_ref.id) - # compile list of the mac_addresses for vifs sorted by network id - macs = [vif_ref['address'] for vif_ref in - sorted(vif_refs, key=lambda vif_ref: vif_ref['network_id'])] - # get project id from instance - project_id = instance_ref.project_id - # combine prefixes, macs, and project_id into (prefix,mac,p_id) tuples - prefix_mac_tuples = zip(prefixes, macs, [project_id for m in macs]) - # return list containing ipv6 address for each tuple - return [ipv6.to_global(*t) for t in prefix_mac_tuples] - - @require_context def instance_get_fixed_addresses_v6(context, instance_id): session = get_session() with session.begin(): # get instance instance_ref = instance_get(context, instance_id, session=session) - return _ipv6_get_by_instance_ref(context, instance_ref) + # assume instance has 1 mac for each network associated with it + # get networks associated with instance + network_refs = network_get_all_by_instance(context, instance_id) + # compile a list of cidr_v6 prefixes sorted by network id + prefixes = [ref.cidr_v6 for ref in + sorted(network_refs, key=lambda ref: ref.id)] + # get vifs associated with instance + vif_refs = virtual_interface_get_by_instance(context, instance_ref.id) + # compile list of the mac_addresses for vifs sorted by network id + macs = [vif_ref['address'] for vif_ref in + sorted(vif_refs, key=lambda vif_ref: vif_ref['network_id'])] + # get project id from instance + project_id = instance_ref.project_id + # combine prefixes, macs, and project_id into (prefix,mac,p_id) tuples + prefix_mac_tuples = zip(prefixes, macs, [project_id for m in macs]) + # return list containing ipv6 address for each tuple + return [ipv6.to_global(*t) for t in prefix_mac_tuples] @require_context -- cgit From ace9aa5d91d839f66998c39a977857b7a7c466a4 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 6 Jul 2011 08:25:28 -0700 Subject: wrap list comparison in test with set()s --- nova/tests/api/openstack/test_servers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 2f466f561..cfb1f9382 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1170,8 +1170,8 @@ class ServersTest(test.TestCase): def fake_get_all(compute_self, context, search_opts=None): self.assertNotEqual(search_opts, None) self.assertTrue('state' in search_opts) - self.assertEqual(search_opts['state'], - [power_state.RUNNING, power_state.BLOCKED]) + self.assertEqual(set(search_opts['state']), + set([power_state.RUNNING, power_state.BLOCKED])) return [stub_instance(100)] self.stubs.Set(nova.compute.API, 'get_all', fake_get_all) -- cgit From 09772f5bf3140a6f4cbaace50ead8d25a874cbb0 Mon Sep 17 00:00:00 2001 From: Jake Dahn Date: Fri, 5 Aug 2011 14:37:44 -0700 Subject: If ip is deallocated from project, but attached to a fixed ip, it is now detached --- nova/api/openstack/contrib/floating_ips.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/nova/api/openstack/contrib/floating_ips.py b/nova/api/openstack/contrib/floating_ips.py index 3d8049324..616388e80 100644 --- a/nova/api/openstack/contrib/floating_ips.py +++ b/nova/api/openstack/contrib/floating_ips.py @@ -97,8 +97,13 @@ class FloatingIPController(object): def delete(self, req, id): context = req.environ['nova.context'] - ip = self.network_api.get_floating_ip(context, id) + try: + if 'fixed_ip' in ip: + self.disassociate(req, id, '') + except: + pass + self.network_api.release_floating_ip(context, address=ip) return {'released': { -- cgit From ccea6c91b2314311587466d67d20f1583ddba1ee Mon Sep 17 00:00:00 2001 From: Jake Dahn Date: Fri, 5 Aug 2011 15:28:10 -0700 Subject: adding logging to exception in delete method --- nova/api/openstack/contrib/floating_ips.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/nova/api/openstack/contrib/floating_ips.py b/nova/api/openstack/contrib/floating_ips.py index 616388e80..49ab88bb6 100644 --- a/nova/api/openstack/contrib/floating_ips.py +++ b/nova/api/openstack/contrib/floating_ips.py @@ -24,6 +24,9 @@ from nova.api.openstack import faults from nova.api.openstack import extensions +LOG = logging.getLogger('nova.api.openstack.contrib.floating_ips') + + def _translate_floating_ip_view(floating_ip): result = {'id': floating_ip['id'], 'ip': floating_ip['address']} @@ -98,11 +101,12 @@ class FloatingIPController(object): def delete(self, req, id): context = req.environ['nova.context'] ip = self.network_api.get_floating_ip(context, id) + try: if 'fixed_ip' in ip: self.disassociate(req, id, '') - except: - pass + except Exception, e: + LOG.exception(_("Error disassociating fixed_ip %s"), e) self.network_api.release_floating_ip(context, address=ip) -- cgit From fe7f229c8ad91b1ae9187b8c541fdefd535eed9b Mon Sep 17 00:00:00 2001 From: Jake Dahn Date: Fri, 5 Aug 2011 16:20:53 -0700 Subject: moving try/except block, and changing syntax of except statement --- nova/api/openstack/contrib/floating_ips.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/nova/api/openstack/contrib/floating_ips.py b/nova/api/openstack/contrib/floating_ips.py index 49ab88bb6..996a42abe 100644 --- a/nova/api/openstack/contrib/floating_ips.py +++ b/nova/api/openstack/contrib/floating_ips.py @@ -101,12 +101,12 @@ class FloatingIPController(object): def delete(self, req, id): context = req.environ['nova.context'] ip = self.network_api.get_floating_ip(context, id) - - try: - if 'fixed_ip' in ip: + + if 'fixed_ip' in ip: + try: self.disassociate(req, id, '') - except Exception, e: - LOG.exception(_("Error disassociating fixed_ip %s"), e) + except Exception as e: + LOG.exception(_("Error disassociating fixed_ip %s"), e) self.network_api.release_floating_ip(context, address=ip) -- cgit From c600b2cf3697fc3587fe5519fda8dd4b82d67234 Mon Sep 17 00:00:00 2001 From: Jake Dahn Date: Mon, 8 Aug 2011 12:10:14 -0700 Subject: adding forgotten import for logging --- nova/api/openstack/contrib/floating_ips.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nova/api/openstack/contrib/floating_ips.py b/nova/api/openstack/contrib/floating_ips.py index 996a42abe..52c9c6cf9 100644 --- a/nova/api/openstack/contrib/floating_ips.py +++ b/nova/api/openstack/contrib/floating_ips.py @@ -18,6 +18,7 @@ from webob import exc from nova import exception +from nova import log as logging from nova import network from nova import rpc from nova.api.openstack import faults -- cgit From 47229cb10c7a322755d36229649c9d3e5712592d Mon Sep 17 00:00:00 2001 From: Johannes Erdfelt Date: Tue, 9 Aug 2011 17:32:39 +0000 Subject: Be more tolerant of agent failures. The instance still booted (most likely) so don't treat it like it didn't --- nova/virt/xenapi/vmops.py | 90 ++++++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 44 deletions(-) diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index b913e764e..50aa0d3b2 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -282,6 +282,7 @@ class VMOps(object): 'architecture': instance.architecture}) def _check_agent_version(): + LOG.debug(_("Querying agent version")) if instance.os_type == 'windows': # Windows will generally perform a setup process on first boot # that can take a couple of minutes and then reboot. So we @@ -292,7 +293,6 @@ class VMOps(object): else: version = self.get_agent_version(instance) if not version: - LOG.info(_('No agent version returned by instance')) return LOG.info(_('Instance agent version: %s') % version) @@ -327,6 +327,10 @@ class VMOps(object): LOG.debug(_("Setting admin password")) self.set_admin_password(instance, admin_password) + def _reset_network(): + LOG.debug(_("Resetting network")) + self.reset_network(instance, vm_ref) + # NOTE(armando): Do we really need to do this in virt? # NOTE(tr3buchet): not sure but wherever we do it, we need to call # reset_network afterwards @@ -341,7 +345,7 @@ class VMOps(object): _check_agent_version() _inject_files() _set_admin_password() - self.reset_network(instance, vm_ref) + _reset_network() return True except Exception, exc: LOG.warn(exc) @@ -597,13 +601,13 @@ class VMOps(object): transaction_id = str(uuid.uuid4()) args = {'id': transaction_id} resp = self._make_agent_call('version', instance, '', args) - if resp is None: - # No response from the agent - return - resp_dict = json.loads(resp) + if resp['returncode'] != '0': + LOG.error(_('Failed to query agent version: %(resp)r') % + locals()) + return None # Some old versions of the Windows agent have a trailing \\r\\n # (ie CRLF escaped) for some reason. Strip that off. - return resp_dict['message'].replace('\\r\\n', '') + return resp['message'].replace('\\r\\n', '') if timeout: vm_ref = self._get_vm_opaque_ref(instance) @@ -634,13 +638,10 @@ class VMOps(object): transaction_id = str(uuid.uuid4()) args = {'id': transaction_id, 'url': url, 'md5sum': md5sum} resp = self._make_agent_call('agentupdate', instance, '', args) - if resp is None: - # No response from the agent - return - resp_dict = json.loads(resp) - if resp_dict['returncode'] != '0': - raise RuntimeError(resp_dict['message']) - return resp_dict['message'] + if resp['returncode'] != '0': + LOG.error(_('Failed to update agent: %(resp)r') % locals()) + return None + return resp['message'] def set_admin_password(self, instance, new_pass): """Set the root/admin password on the VM instance. @@ -659,18 +660,13 @@ class VMOps(object): key_init_args = {'id': key_init_transaction_id, 'pub': str(dh.get_public())} resp = self._make_agent_call('key_init', instance, '', key_init_args) - if resp is None: - # No response from the agent - return - resp_dict = json.loads(resp) # Successful return code from key_init is 'D0' - if resp_dict['returncode'] != 'D0': - # There was some sort of error; the message will contain - # a description of the error. - raise RuntimeError(resp_dict['message']) + if resp['returncode'] != 'D0': + LOG.error(_('Failed to exchange keys: %(resp)r') % locals()) + return None # Some old versions of the Windows agent have a trailing \\r\\n # (ie CRLF escaped) for some reason. Strip that off. - agent_pub = int(resp_dict['message'].replace('\\r\\n', '')) + agent_pub = int(resp['message'].replace('\\r\\n', '')) dh.compute_shared(agent_pub) # Some old versions of Linux and Windows agent expect trailing \n # on password to work correctly. @@ -679,17 +675,14 @@ class VMOps(object): password_transaction_id = str(uuid.uuid4()) password_args = {'id': password_transaction_id, 'enc_pass': enc_pass} resp = self._make_agent_call('password', instance, '', password_args) - if resp is None: - # No response from the agent - return - resp_dict = json.loads(resp) # Successful return code from password is '0' - if resp_dict['returncode'] != '0': - raise RuntimeError(resp_dict['message']) + if resp['returncode'] != '0': + LOG.error(_('Failed to update password: %(resp)r') % locals()) + return None db.instance_update(nova_context.get_admin_context(), instance['id'], dict(admin_pass=new_pass)) - return resp_dict['message'] + return resp['message'] def inject_file(self, instance, path, contents): """Write a file to the VM instance. @@ -712,12 +705,10 @@ class VMOps(object): # If the agent doesn't support file injection, a NotImplementedError # will be raised with the appropriate message. resp = self._make_agent_call('inject_file', instance, '', args) - resp_dict = json.loads(resp) - if resp_dict['returncode'] != '0': - # There was some other sort of error; the message will contain - # a description of the error. - raise RuntimeError(resp_dict['message']) - return resp_dict['message'] + if resp['returncode'] != '0': + LOG.error(_('Failed to inject file: %(resp)r') % locals()) + return None + return resp['message'] def _shutdown(self, instance, vm_ref, hard=True): """Shutdown an instance.""" @@ -1178,8 +1169,19 @@ class VMOps(object): def _make_agent_call(self, method, vm, path, addl_args=None): """Abstracts out the interaction with the agent xenapi plugin.""" - return self._make_plugin_call('agent', method=method, vm=vm, + ret = self._make_plugin_call('agent', method=method, vm=vm, path=path, addl_args=addl_args) + if isinstance(ret, dict): + return ret + try: + return json.loads(ret) + except TypeError: + instance_id = vm.id + LOG.error(_('The agent call to %(method)s returned an invalid' + ' response: %(ret)r. VM id=%(instance_id)s;' + ' path=%(path)s; args=%(addl_args)r') % locals()) + return {'returncode': 'error', + 'message': 'unable to deserialize response'} def _make_plugin_call(self, plugin, method, vm, path, addl_args=None, vm_ref=None): @@ -1197,20 +1199,20 @@ class VMOps(object): ret = self._session.wait_for_task(task, instance_id) except self.XenAPI.Failure, e: ret = None - err_trace = e.details[-1] - err_msg = err_trace.splitlines()[-1] - strargs = str(args) + err_msg = e.details[-1].splitlines()[-1] if 'TIMEOUT:' in err_msg: LOG.error(_('TIMEOUT: The call to %(method)s timed out. ' - 'VM id=%(instance_id)s; args=%(strargs)s') % locals()) + 'VM id=%(instance_id)s; args=%(args)r') % locals()) + return {'returncode': 'timeout', 'message': err_msg} elif 'NOT IMPLEMENTED:' in err_msg: LOG.error(_('NOT IMPLEMENTED: The call to %(method)s is not' ' supported by the agent. VM id=%(instance_id)s;' - ' args=%(strargs)s') % locals()) - raise NotImplementedError(err_msg) + ' args=%(args)r') % locals()) + return {'returncode': 'notimplemented', 'message': err_msg} else: LOG.error(_('The call to %(method)s returned an error: %(e)s. ' - 'VM id=%(instance_id)s; args=%(strargs)s') % locals()) + 'VM id=%(instance_id)s; args=%(args)r') % locals()) + return {'returncode': 'error', 'message': err_msg} return ret def add_to_xenstore(self, vm, path, key, value): -- cgit From c95954ca1a704b6f6e53e7b37f797ad51cb5efa9 Mon Sep 17 00:00:00 2001 From: Jake Dahn Date: Tue, 9 Aug 2011 14:17:56 -0700 Subject: adding myself to authors --- Authors | 1 + 1 file changed, 1 insertion(+) diff --git a/Authors b/Authors index b216873df..e639cbf76 100644 --- a/Authors +++ b/Authors @@ -37,6 +37,7 @@ Hisaharu Ishii Hisaki Ohara Ilya Alekseyev Isaku Yamahata +Jake Dahn Jason Cannavale Jason Koelker Jay Pipes -- cgit From cfa2303fcb0b59e64504d079256e4356fa3bf01f Mon Sep 17 00:00:00 2001 From: Jake Dahn Date: Tue, 9 Aug 2011 14:45:31 -0700 Subject: adding other emails to mailmap --- .mailmap | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.mailmap b/.mailmap index 76e7bc669..5c8df80e0 100644 --- a/.mailmap +++ b/.mailmap @@ -18,6 +18,8 @@ + + -- cgit