From 1c90eb34085dbb69f37e2f63dea7496afabb06b3 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 6 Jul 2011 06:20:38 -0700 Subject: [PATCH] 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 966c3a564c0..db49baffa62 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -242,7 +242,7 @@ def get_metadata(self, address): 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 382f3c54150..7a3ff0c5622 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -683,44 +683,49 @@ def get_all(self, context, search_opts=None): 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 @@ def get_all(self, context, search_opts=None): 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 7792f590936..18ec085978d 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -949,23 +949,32 @@ def test_get_by_fixed_ip(self): 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 @@ def test_get_by_fixed_ip(self): 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)