diff --git a/octavia/api/v2/controllers/member.py b/octavia/api/v2/controllers/member.py index 6e9deb3c5e..164e1eff2c 100644 --- a/octavia/api/v2/controllers/member.py +++ b/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) diff --git a/octavia/tests/functional/api/v2/test_member.py b/octavia/tests/functional/api/v2/test_member.py index 700bfea281..fcf058895a 100644 --- a/octavia/tests/functional/api/v2/test_member.py +++ b/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): diff --git a/releasenotes/notes/catch_validation-27ffe48ca187c46f.yaml b/releasenotes/notes/catch_validation-27ffe48ca187c46f.yaml new file mode 100644 index 0000000000..73a2045ff0 --- /dev/null +++ b/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.