Use neutron port_list when filtering instance by ip

The performance of filtering instance by IP is poor, due to the fact that
the IP address is part of a JSON-encoded filed and we need to unpack the
field and do a search on the field for each record in instances table, we
have to iterate one by one to find the instance that matches the request.

As discussed in Queens PTG[1], one possible solution is to get filtered
ports from Neutron and retrieve the instance uuid from the port.device_id
and then merge to the other filters.

As Nova provides regex matching manner filtering for IP filter, so this
depend on Neutron changes that add substring matching functionality to
the GET /ports API[2]

The performance test results of the POC are presented in[3].

[1]http://lists.openstack.org/pipermail/openstack-dev/2017-September/122258.html
[2]https://bugs.launchpad.net/neutron/+bug/1718605
[3]http://lists.openstack.org/pipermail/openstack-dev/2018-January/125990.html

Depends-On: I9549b2ba676e1bad0812682c3f3f3c97de15f5f6

Co-Authored-By: Matt Riedemann <mriedem.os@gmail.com>

Implements: blueprint improve-filter-instances-by-ip-performance

Change-Id: I06aabfdce4aaefde080c7e122552ce4f36da5804

nova/compute/api.py

@@ -2347,7 +2347,34 @@ class API(base.Base):
         # limit so that it can be applied after the IP filter.
         filter_ip = 'ip6' in filters or 'ip' in filters
         orig_limit = limit
-        if filter_ip and limit:
+        if filter_ip:
+            if self.network_api.has_substr_port_filtering_extension(context):
+                # We're going to filter by IP using Neutron so set filter_ip
+                # to False so we don't attempt post-DB query filtering in
+                # memory below.
+                filter_ip = False
+                instance_uuids = self._ip_filter_using_neutron(context,
+                                                               filters)
+                if instance_uuids:
+                    # Note that 'uuid' is not in the 2.1 GET /servers query
+                    # parameter schema, however, we allow additionalProperties
+                    # so someone could filter instances by uuid, which doesn't
+                    # make a lot of sense but we have to account for it.
+                    if 'uuid' in filters and filters['uuid']:
+                        filter_uuids = filters['uuid']
+                        if isinstance(filter_uuids, list):
+                            instance_uuids.extend(filter_uuids)
+                        else:
+                            # Assume a string. If it's a dict or tuple or
+                            # something, well...that's too bad. This is why
+                            # we have query parameter schema definitions.
+                            if filter_uuids not in instance_uuids:
+                                instance_uuids.append(filter_uuids)
+                    filters['uuid'] = instance_uuids
+                else:
+                    # No matches on the ip filter(s), return an empty list.
+                    return objects.InstanceList()
+            elif limit:
                 LOG.debug('Removing limit for DB query due to IP filter')
                 limit = None
 
@@ -2484,6 +2511,26 @@ class API(base.Base):
                     break
         return objects.InstanceList(objects=result_objs)
 
+    def _ip_filter_using_neutron(self, context, filters):
+        ip4_address = filters.get('ip')
+        ip6_address = filters.get('ip6')
+        addresses = [ip4_address, ip6_address]
+        uuids = []
+        for address in addresses:
+            if address:
+                try:
+                    ports = self.network_api.list_ports(
+                        context, fixed_ips='ip_address_substr=' + address,
+                        fields=['device_id'])['ports']
+                    for port in ports:
+                        uuids.append(port['device_id'])
+                except Exception as e:
+                    LOG.error('An error occurred while listing ports '
+                              'with an ip_address filter value of "%s". '
+                              'Error: %s',
+                              address, six.text_type(e))
+        return uuids
+
     def _get_instances_by_filters(self, context, filters,
                                   limit=None, marker=None, fields=None,
                                   sort_keys=None, sort_dirs=None):

nova/network/base_api.py

@@ -367,3 +367,6 @@ class NetworkAPI(base.Base):
         :param index: The index on the instance for the VIF.
         """
         pass
+
+    def has_substr_port_filtering_extension(self, context):
+        return False

nova/network/neutronv2/api.py

@@ -1112,6 +1112,10 @@ class API(base_api.NetworkAPI):
         self._refresh_neutron_extensions_cache(context, neutron=neutron)
         return constants.QOS_QUEUE in self.extensions
 
+    def has_substr_port_filtering_extension(self, context):
+        self._refresh_neutron_extensions_cache(context)
+        return constants.SUBSTR_PORT_FILTERING in self.extensions
+
     def _get_pci_device_profile(self, pci_dev):
         dev_spec = self.pci_whitelist.get_devspec(pci_dev)
         if dev_spec:

nova/network/neutronv2/constants.py

@@ -18,3 +18,4 @@ NET_EXTERNAL = 'router:external'
 VNIC_INDEX_EXT = 'VNIC Index'
 DNS_INTEGRATION = 'DNS Integration'
 MULTI_NET_EXT = 'Multi Provider Network'
+SUBSTR_PORT_FILTERING = 'IP address substring filtering'

nova/tests/unit/compute/test_compute.py

@@ -11374,7 +11374,10 @@ class ComputeAPIIpFilterTestCase(test.NoDBTestCase):
     @mock.patch.object(objects.BuildRequestList, 'get_by_filters')
     @mock.patch.object(objects.CellMapping, 'get_by_uuid',
                        side_effect=exception.CellMappingNotFound(uuid='fake'))
-    def test_ip_filtering_no_limit_to_db(self, _mock_cell_map_get,
+    @mock.patch('nova.network.neutronv2.api.API.'
+                'has_substr_port_filtering_extension', return_value=False)
+    def test_ip_filtering_no_limit_to_db(self, mock_has_port_filter_ext,
+                                         _mock_cell_map_get,
                                          mock_buildreq_get):
         c = context.get_admin_context()
         # Limit is not supplied to the DB when using an IP filter

nova/tests/unit/compute/test_compute_api.py

@@ -38,6 +38,7 @@ from nova import conductor
 from nova import context
 from nova import db
 from nova import exception
+from nova.network.neutronv2 import api as neutron_api
 from nova import objects
 from nova.objects import base as obj_base
 from nova.objects import block_device as block_device_obj
@@ -5630,6 +5631,123 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
                               self.compute_api.attach_volume,
                               self.context, instance, uuids.volumeid)
 
+    @mock.patch.object(neutron_api.API, 'has_substr_port_filtering_extension')
+    @mock.patch.object(neutron_api.API, 'list_ports')
+    @mock.patch.object(objects.BuildRequestList, 'get_by_filters')
+    def test_get_all_ip_filter_use_neutron(self, mock_buildreq_get,
+                                           mock_list_port, mock_check_ext):
+        mock_check_ext.return_value = True
+        cell_instances = self._list_of_instances(2)
+        mock_list_port.return_value = {
+            'ports': [{'device_id': 'fake_device_id'}]}
+        with mock.patch('nova.compute.instance_list.'
+                        'get_instance_objects_sorted') as mock_inst_get:
+            mock_inst_get.return_value = objects.InstanceList(
+                self.context, objects=cell_instances)
+
+            self.compute_api.get_all(
+                self.context, search_opts={'ip': 'fake'},
+                limit=None, marker='fake-marker', sort_keys=['baz'],
+                sort_dirs=['desc'])
+
+            mock_list_port.assert_called_once_with(
+                self.context, fixed_ips='ip_address_substr=fake',
+                fields=['device_id'])
+            mock_buildreq_get.assert_called_once_with(
+                self.context, {'ip': 'fake', 'uuid': ['fake_device_id']},
+                limit=None, marker='fake-marker',
+                sort_keys=['baz'], sort_dirs=['desc'])
+            fields = ['metadata', 'info_cache', 'security_groups']
+            mock_inst_get.assert_called_once_with(
+                self.context, {'ip': 'fake', 'uuid': ['fake_device_id']},
+                None, None, fields, ['baz'], ['desc'])
+
+    @mock.patch.object(neutron_api.API, 'has_substr_port_filtering_extension')
+    @mock.patch.object(neutron_api.API, 'list_ports')
+    @mock.patch.object(objects.BuildRequestList, 'get_by_filters')
+    def test_get_all_ip6_filter_use_neutron(self, mock_buildreq_get,
+                                            mock_list_port, mock_check_ext):
+        mock_check_ext.return_value = True
+        cell_instances = self._list_of_instances(2)
+        mock_list_port.return_value = {
+            'ports': [{'device_id': 'fake_device_id'}]}
+        with mock.patch('nova.compute.instance_list.'
+                        'get_instance_objects_sorted') as mock_inst_get:
+            mock_inst_get.return_value = objects.InstanceList(
+                self.context, objects=cell_instances)
+
+            self.compute_api.get_all(
+                self.context, search_opts={'ip6': 'fake'},
+                limit=None, marker='fake-marker', sort_keys=['baz'],
+                sort_dirs=['desc'])
+
+            mock_list_port.assert_called_once_with(
+                self.context, fixed_ips='ip_address_substr=fake',
+                fields=['device_id'])
+            mock_buildreq_get.assert_called_once_with(
+                self.context, {'ip6': 'fake', 'uuid': ['fake_device_id']},
+                limit=None, marker='fake-marker',
+                sort_keys=['baz'], sort_dirs=['desc'])
+            fields = ['metadata', 'info_cache', 'security_groups']
+            mock_inst_get.assert_called_once_with(
+                self.context, {'ip6': 'fake', 'uuid': ['fake_device_id']},
+                None, None, fields, ['baz'], ['desc'])
+
+    @mock.patch.object(neutron_api.API, 'has_substr_port_filtering_extension')
+    @mock.patch.object(neutron_api.API, 'list_ports')
+    @mock.patch.object(objects.BuildRequestList, 'get_by_filters')
+    def test_get_all_ip_and_ip6_filter_use_neutron(self, mock_buildreq_get,
+                                                   mock_list_port,
+                                                   mock_check_ext):
+        mock_check_ext.return_value = True
+        cell_instances = self._list_of_instances(2)
+        mock_list_port.return_value = {
+            'ports': [{'device_id': 'fake_device_id'}]}
+        with mock.patch('nova.compute.instance_list.'
+                        'get_instance_objects_sorted') as mock_inst_get:
+            mock_inst_get.return_value = objects.InstanceList(
+                self.context, objects=cell_instances)
+
+            self.compute_api.get_all(
+                self.context, search_opts={'ip': 'fake1', 'ip6': 'fake2'},
+                limit=None, marker='fake-marker', sort_keys=['baz'],
+                sort_dirs=['desc'])
+
+            mock_list_port.assert_has_calls([
+                mock.call(
+                    self.context, fixed_ips='ip_address_substr=fake1',
+                    fields=['device_id']),
+                mock.call(
+                    self.context, fixed_ips='ip_address_substr=fake2',
+                    fields=['device_id'])
+            ])
+            mock_buildreq_get.assert_called_once_with(
+                self.context, {'ip': 'fake1', 'ip6': 'fake2',
+                               'uuid': ['fake_device_id', 'fake_device_id']},
+                limit=None, marker='fake-marker',
+                sort_keys=['baz'], sort_dirs=['desc'])
+            fields = ['metadata', 'info_cache', 'security_groups']
+            mock_inst_get.assert_called_once_with(
+                self.context, {'ip': 'fake1', 'ip6': 'fake2',
+                               'uuid': ['fake_device_id', 'fake_device_id']},
+                None, None, fields, ['baz'], ['desc'])
+
+    @mock.patch.object(neutron_api.API, 'has_substr_port_filtering_extension')
+    @mock.patch.object(neutron_api.API, 'list_ports')
+    def test_get_all_ip6_filter_use_neutron_exc(self, mock_list_port,
+                                                mock_check_ext):
+        mock_check_ext.return_value = True
+        mock_list_port.side_effect = exception.InternalError('fake')
+
+        instances = self.compute_api.get_all(
+            self.context, search_opts={'ip6': 'fake'},
+            limit=None, marker='fake-marker', sort_keys=['baz'],
+            sort_dirs=['desc'])
+        mock_list_port.assert_called_once_with(
+            self.context, fixed_ips='ip_address_substr=fake',
+            fields=['device_id'])
+        self.assertEqual([], instances.objects)
+
 
 class Cellsv1DeprecatedTestMixIn(object):
     @mock.patch.object(objects.BuildRequestList, 'get_by_filters')

releasenotes/notes/use-neutron-when-list-instances-by-ip-6682018bf88b6b0e.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - When ``IP address substring filtering`` extension
+    is available in Neutron, Nova will proxy the instance
+    list with ``ip`` or ``ip6`` filter to Neutron for
+    better performance.