From 38b2f68a17533e839819e654825613aefd4effd4 Mon Sep 17 00:00:00 2001 From: Doug Wiegley Date: Tue, 5 Nov 2019 17:29:11 -0500 Subject: [PATCH] Improve metadata server performance with large security groups Don't include the rules in the SG fetch in the metadata server, since we don't need them there, and with >1000 rules, it starts to get really slow, especially in Pike and later. Closes-Bug: #1851430 Co-Authored-By: Doug Wiegley Co-Authored-By: Matt Riedemann Change-Id: I7de14456d04370c842b4c35597dca3a628a826a2 (cherry picked from commit eaf16fdde59a14fb38df669b21a911a0c2d2576f) (cherry picked from commit 418af2d865809cfa907678f883dae07f4f31baa2) (cherry picked from commit fec95a2e4f763e15193504483383f918feb3e636) --- nova/network/security_group/neutron_driver.py | 16 ++++++-- .../security_group/test_neutron_driver.py | 40 +++++++++++++++---- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/nova/network/security_group/neutron_driver.py b/nova/network/security_group/neutron_driver.py index 2a55d473f6e5..843c954afbe2 100644 --- a/nova/network/security_group/neutron_driver.py +++ b/nova/network/security_group/neutron_driver.py @@ -342,7 +342,7 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase): return ports - def _get_secgroups_from_port_list(self, ports, neutron): + def _get_secgroups_from_port_list(self, ports, neutron, fields=None): """Returns a dict of security groups keyed by their ids.""" def _chunk_by_ids(sg_ids, limit): @@ -365,6 +365,8 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase): security_groups = {} for sg_id_list in _chunk_by_ids(sg_ids, MAX_SEARCH_IDS): sg_search_opts = {'id': sg_id_list} + if fields: + sg_search_opts['fields'] = fields search_results = neutron.list_security_groups(**sg_search_opts) for sg in search_results.get('security_groups'): security_groups[sg['id']] = sg @@ -375,13 +377,20 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase): detailed=False): """Returns a dict(instance_id, [security_groups]) to allow obtaining all of the instances and their security groups in one shot. + If detailed is False only the security group name is returned. """ neutron = neutronapi.get_client(context) ports = self._get_ports_from_server_list(servers, neutron) - security_groups = self._get_secgroups_from_port_list(ports, neutron) + # If detailed is True, we want all fields from the security groups + # including the potentially slow-to-join security_group_rules field. + # But if detailed is False, only get the id and name fields since + # that's all we'll use below. + fields = None if detailed else ['id', 'name'] + security_groups = self._get_secgroups_from_port_list( + ports, neutron, fields=fields) instances_security_group_bindings = {} for port in ports: @@ -411,7 +420,8 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase): def get_instance_security_groups(self, context, instance, detailed=False): """Returns the security groups that are associated with an instance. If detailed is True then it also returns the full details of the - security groups associated with an instance. + security groups associated with an instance, otherwise just the + security group name. """ servers = [{'id': instance.uuid}] sg_bindings = self.get_instances_security_groups_bindings( diff --git a/nova/tests/unit/network/security_group/test_neutron_driver.py b/nova/tests/unit/network/security_group/test_neutron_driver.py index 3c861e2e3730..c95b383f23b1 100644 --- a/nova/tests/unit/network/security_group/test_neutron_driver.py +++ b/nova/tests/unit/network/security_group/test_neutron_driver.py @@ -272,7 +272,7 @@ class TestNeutronDriver(test.NoDBTestCase): self.assertEqual(expected, result) self.mocked_client.list_security_groups.assert_called_once_with() - def test_instances_security_group_bindings(self): + def test_instances_security_group_bindings(self, detailed=False): server_id = 'c5a20e8d-c4b0-47cf-9dca-ebe4f758acb1' port1_id = '4c505aec-09aa-47bc-bcc0-940477e84dc0' port2_id = 'b3b31a53-6e29-479f-ae5c-00b7b71a6d44' @@ -288,23 +288,47 @@ class TestNeutronDriver(test.NoDBTestCase): sg2 = {'id': sg2_id, 'name': 'eor'} security_groups_list = {'security_groups': [sg1, sg2]} - sg_bindings = {server_id: [{'name': 'wol'}, {'name': 'eor'}]} - self.mocked_client.list_ports.return_value = port_list self.mocked_client.list_security_groups.return_value = ( security_groups_list) + def stub_convert(sg): + if sg == sg1: + return mock.sentinel.sg1 + if sg == sg2: + return mock.sentinel.sg2 + raise Exception("Unexpected security group in " + "_convert_to_nova_security_group_format: %s" % sg) + sg_api = neutron_driver.SecurityGroupAPI() - result = sg_api.get_instances_security_groups_bindings( - self.context, servers) + with mock.patch.object( + sg_api, '_convert_to_nova_security_group_format', + side_effect=stub_convert) as convert: + result = sg_api.get_instances_security_groups_bindings( + self.context, servers, detailed=detailed) + if detailed: + convert.assert_has_calls([mock.call(sg1), mock.call(sg2)], + any_order=True) + sg_bindings = {server_id: [ + mock.sentinel.sg1, mock.sentinel.sg2 + ]} + else: + convert.assert_not_called() + sg_bindings = {server_id: [{'name': 'wol'}, {'name': 'eor'}]} self.assertEqual(sg_bindings, result) self.mocked_client.list_ports.assert_called_once_with( device_id=[server_id]) + expected_search_opts = {'id': mock.ANY} + if not detailed: + expected_search_opts['fields'] = ['id', 'name'] self.mocked_client.list_security_groups.assert_called_once_with( - id=mock.ANY) + **expected_search_opts) self.assertEqual(sorted([sg1_id, sg2_id]), sorted(self.mocked_client.list_security_groups.call_args[1]['id'])) + def test_instances_security_group_bindings_detailed(self): + self.test_instances_security_group_bindings(detailed=True) + def test_instances_security_group_bindings_port_not_found(self): server_id = 'c5a20e8d-c4b0-47cf-9dca-ebe4f758acb1' servers = [{'id': server_id}] @@ -355,7 +379,7 @@ class TestNeutronDriver(test.NoDBTestCase): self.context, servers) self.assertEqual(sg_bindings, result) self.mocked_client.list_security_groups.assert_called_once_with( - id=mock.ANY) + id=mock.ANY, fields=['id', 'name']) self.assertEqual(sorted([sg1_id, sg2_id]), sorted(self.mocked_client.list_security_groups.call_args[1]['id'])) self.assertEqual(expected_args, @@ -392,7 +416,7 @@ class TestNeutronDriver(test.NoDBTestCase): self.mocked_client.list_ports.assert_called_once_with( device_id=['server_1']) self.mocked_client.list_security_groups.assert_called_once_with( - id=mock.ANY) + id=mock.ANY, fields=['id', 'name']) self.assertEqual(['1', '2'], sorted(self.mocked_client.list_security_groups.call_args[1]['id']))