Merge "Improve metadata server performance with large security groups…

…" into stable/stein

nova/network/security_group/neutron_driver.py

@@ -342,7 +342,7 @@ def _chunk_by_ids(servers, limit):
 
         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 @@ def _chunk_by_ids(sg_ids, limit):
         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 @@ def get_instances_security_groups_bindings(self, context, servers,
                                                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 @@ def get_instances_security_groups_bindings(self, context, servers,
     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(

nova/tests/unit/network/security_group/test_neutron_driver.py

@@ -272,7 +272,7 @@ def test_list_security_group_with_no_port_range_and_not_tcp_udp_icmp(self):
         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 @@ def test_instances_security_group_bindings(self):
         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 @@ def _test_instances_security_group_bindings_scale(self, num_servers):
                                   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 @@ def test_instances_security_group_bindings_with_hidden_sg(self):
         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']))