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 <dougwig@parkside.io>
Co-Authored-By: Matt Riedemann <mriedem.os@gmail.com>

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

NOTE(mriedem): The conflict is due to not having change
I31c9ea8628c6f3985f8e9118d9687bbfb8789b68 in Pike.

Change-Id: I7de14456d04370c842b4c35597dca3a628a826a2
(cherry picked from commit eaf16fdde59a14fb38df669b21a911a0c2d2576f)
(cherry picked from commit 418af2d865809cfa907678f883dae07f4f31baa2)
(cherry picked from commit fec95a2e4f763e15193504483383f918feb3e636)
(cherry picked from commit 38b2f68a17533e839819e654825613aefd4effd4)
(cherry picked from commit 00d438adb325610a04af9f8f18cdb1c622df5418)

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 b6b558b8c7..9b8bd41adb 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 99a9f109d2..e9c1f1ee3d 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']))