Merge "Cache subnets validation for batch member update" into stable/victoria

octavia/api/v2/controllers/member.py

@@ -336,13 +336,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)
 
@@ -388,8 +381,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.