Cache subnets validation for batch member update

At the moment, we are actually going ahead and sending
an API request for every single member when doing a batch
update, this can be very expensive in large number of
members and will cause a big hit against the Neutron API,
it potentially also makes those API calls timeout.

This patch instead implements a simple cache (per batch
call) to simply store the subnets which are considered valid
which means that it will make significantly less calls,
potentially even one.

It also adjusts the behaviour so that it only runs this on newly
added members which should avoid running it for deleted and
updated members which won't have an effect in this change.

Change-Id: I24ec7dc3b18111c126bcfdaa0780bfd7993020fb

octavia/api/v2/controllers/member.py

@@ -337,13 +337,6 @@ class MembersController(MemberController):
             self._auth_validate_action(context, project_id,
                                        constants.RBAC_DELETE)
 
-        # Validate member subnets
-        for member in members:
-            if member.subnet_id and not validate.subnet_exists(
-                    member.subnet_id, context=context):
-                raise exceptions.NotFound(resource='Subnet',
-                                          id=member.subnet_id)
-
         # Load the driver early as it also provides validation
         driver = driver_factory.get_driver(provider)
 
@@ -389,8 +382,27 @@ class MembersController(MemberController):
                     resource=data_models.Member._name())
 
             provider_members = []
+            valid_subnets = set()
             # Create new members
             for m in new_members:
+                # NOTE(mnaser): In order to avoid hitting the Neutron API hard
+                # when creating many new members, we cache the
+                # validation results. We also validate new
+                # members only since subnet ID is immutable.
+                # If the member doesn't have a subnet, or the subnet is
+                # already valid, move on. Run validate and add it to
+                # cache otherwise.
+                if m.subnet_id and m.subnet_id not in valid_subnets:
+                    # If the subnet does not exist,
+                    # raise an exception and get out.
+                    if not validate.subnet_exists(
+                            m.subnet_id, context=context):
+                        raise exceptions.NotFound(
+                            resource='Subnet', id=m.subnet_id)
+
+                    # Mark the subnet as valid for future runs.
+                    valid_subnets.add(m.subnet_id)
+
                 m = m.to_dict(render_unsets=False)
                 m['project_id'] = db_pool.project_id
                 created_member = self._graph_create(lock_session, m)

octavia/tests/functional/api/v2/test_member.py

@@ -882,6 +882,50 @@ class TestMember(base.BaseAPITest):
                                               mock_driver.member_batch_update,
                                               self.pool_id, provider_members)
 
+    @mock.patch('octavia.api.drivers.driver_factory.get_driver')
+    @mock.patch('octavia.api.drivers.utils.call_provider')
+    def test_update_members_subnet_duplicate(
+            self, mock_provider, mock_get_driver):
+        mock_driver = mock.MagicMock()
+        mock_driver.name = 'noop_driver'
+        mock_get_driver.return_value = mock_driver
+        subnet_id = uuidutils.generate_uuid()
+
+        member1 = {'address': '192.0.2.1', 'protocol_port': 80,
+                   'project_id': self.project_id, 'subnet_id': subnet_id}
+        member2 = {'address': '192.0.2.2', 'protocol_port': 80,
+                   'project_id': self.project_id, 'subnet_id': subnet_id}
+
+        req_dict = [member1, member2]
+        body = {self.root_tag_list: req_dict}
+        path = self.MEMBERS_PATH.format(pool_id=self.pool_id)
+        with mock.patch("octavia.common.validate."
+                        "subnet_exists") as m_subnet_exists:
+            m_subnet_exists.return_value = True
+            self.put(path, body, status=202)
+            m_subnet_exists.assert_called_once_with(
+                member1['subnet_id'], context=mock.ANY)
+
+    @mock.patch('octavia.api.drivers.driver_factory.get_driver')
+    @mock.patch('octavia.api.drivers.utils.call_provider')
+    def test_update_members_subnet_not_found(
+            self, mock_provider, mock_get_driver):
+        mock_driver = mock.MagicMock()
+        mock_driver.name = 'noop_driver'
+        mock_get_driver.return_value = mock_driver
+        fake_subnet_id = uuidutils.generate_uuid()
+
+        member1 = {'address': '192.0.2.1', 'protocol_port': 80,
+                   'project_id': self.project_id, 'subnet_id': fake_subnet_id}
+
+        req_dict = [member1]
+        body = {self.root_tag_list: req_dict}
+        path = self.MEMBERS_PATH.format(pool_id=self.pool_id)
+        with mock.patch("octavia.common.validate."
+                        "subnet_exists") as m_subnet_exists:
+            m_subnet_exists.return_value = False
+            self.put(path, body, status=404)
+
     @mock.patch('octavia.api.drivers.driver_factory.get_driver')
     @mock.patch('octavia.api.drivers.utils.call_provider')
     def test_delete_batch_members(self, mock_provider, mock_get_driver):

releasenotes/notes/catch_validation-27ffe48ca187c46f.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    In order to avoid hitting the Neutron API hard
+    when batch update with creating many new members, we cache the
+    subnet validation results in batch update members API call.
+    We also change to validate new members only during batch update
+    members since subnet ID is immutable.