diff --git a/nova/network/security_group/neutron_driver.py b/nova/network/security_group/neutron_driver.py index b6b558b8c7f9..9b8bd41adbd4 100644 --- a/nova/network/security_group/neutron_driver.py +++ b/nova/network/security_group/neutron_driver.py @@ -338,7 +338,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): @@ -361,6 +361,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 @@ -371,13 +373,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: @@ -407,7 +416,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 99a9f109d2a4..e9c1f1ee3dce 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_scale(self, num_servers): max_query = 150 sg1_id = '2f7ce969-1a73-4ef9-bbd6-c9a91780ecd4' @@ -342,7 +366,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, @@ -379,7 +403,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']))