clean up compute_api.get_all filter name remappings. ditch fixed_ip one-off code. fixed ec2 api call to this to compensate

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)