From 3f35fe6a88672ea2ab7e080a55235c5cca45dc2c Mon Sep 17 00:00:00 2001 From: Kevin_Zheng Date: Tue, 5 Dec 2017 16:56:35 +0800 Subject: [PATCH] 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 Implements: blueprint improve-filter-instances-by-ip-performance Change-Id: I06aabfdce4aaefde080c7e122552ce4f36da5804 --- nova/compute/api.py | 53 +++++++- nova/network/base_api.py | 3 + nova/network/neutronv2/api.py | 4 + nova/network/neutronv2/constants.py | 1 + nova/tests/unit/compute/test_compute.py | 5 +- nova/tests/unit/compute/test_compute_api.py | 118 ++++++++++++++++++ ...list-instances-by-ip-6682018bf88b6b0e.yaml | 6 + 7 files changed, 186 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/use-neutron-when-list-instances-by-ip-6682018bf88b6b0e.yaml diff --git a/nova/compute/api.py b/nova/compute/api.py index b366daaa6e..24a4b2d72c 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2347,9 +2347,36 @@ 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: - LOG.debug('Removing limit for DB query due to IP filter') - limit = None + 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 # The ordering of instances will be # [sorted instances with no host] + [sorted instances with host]. @@ -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): diff --git a/nova/network/base_api.py b/nova/network/base_api.py index 23bb186285..d46c571f0b 100644 --- a/nova/network/base_api.py +++ b/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 diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index 2d7088f773..bb7ea3cb8d 100644 --- a/nova/network/neutronv2/api.py +++ b/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: diff --git a/nova/network/neutronv2/constants.py b/nova/network/neutronv2/constants.py index 0b76f57705..91bc686e54 100644 --- a/nova/network/neutronv2/constants.py +++ b/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' diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index ed28a39c54..0d2d5eaa3b 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/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 diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index beead7069b..cedeb35467 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/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') diff --git a/releasenotes/notes/use-neutron-when-list-instances-by-ip-6682018bf88b6b0e.yaml b/releasenotes/notes/use-neutron-when-list-instances-by-ip-6682018bf88b6b0e.yaml new file mode 100644 index 0000000000..b715bd6d6e --- /dev/null +++ b/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.