openstack
/
octavia
Code Issues Proposed changes

Fix batch member update error on empty change list 

If the list of changes was empty, the worker would fail to fetch the
pool because it was retrieved implicitly from one of the changed
members. Pass it explicitly instead, and also short-circuit on NOOPs.

Story: 2006719
Task: 37090

Depends-On: https://review.opendev.org/#/c/688546/
Change-Id: I161a522abad4a2aa521ea46cb1065c5b05a2cd2e
This commit is contained in:
Adam Harwell 2019-10-14 12:43:32 -07:00
parent fce2cd4f49
commit 01dfb8ffaf
9 changed files with 94 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
doc/source/contributor/guides/providers.rst
View File

@ -1111,9 +1111,10 @@ and members in the list that do not already exist should be created.
"""
raise NotImplementedError()
def member_batch_update(self, members):
def member_batch_update(self, pool_id, members):
"""Creates, updates, or deletes a set of pool members.
:param pool_id (string): The id of the pool to update.
:param members (list): List of member objects.
:return: Nothing if the create request was accepted.
:raises DriverError: An unexpected error occurred in the driver.

15
octavia/api/drivers/amphora_driver/v1/driver.py
View File

@ -201,9 +201,7 @@ class AmphoraProviderDriver(driver_base.ProviderDriver):
consts.MEMBER_UPDATES: member_dict}
self.client.cast({}, 'update_member', **payload)
def member_batch_update(self, members):
# Get a list of existing members
pool_id = members[0].pool_id
def member_batch_update(self, pool_id, members):
# The DB should not have updated yet, so we can still use the pool
db_pool = self.repositories.pool.get(db_apis.get_session(), id=pool_id)
@ -236,10 +234,13 @@ class AmphoraProviderDriver(driver_base.ProviderDriver):
if m.id not in new_member_ids:
deleted_members.append(m)
payload = {'old_member_ids': [m.id for m in deleted_members],
'new_member_ids': [m.member_id for m in new_members],
'updated_members': updated_members}
self.client.cast({}, 'batch_update_members', **payload)
if deleted_members or new_members or updated_members:
payload = {'old_member_ids': [m.id for m in deleted_members],
'new_member_ids': [m.member_id for m in new_members],
'updated_members': updated_members}
self.client.cast({}, 'batch_update_members', **payload)
else:
LOG.info("Member batch update is a noop, returning early.")
def _validate_members(self, db_pool, members):
if db_pool.protocol == consts.PROTOCOL_UDP:

15
octavia/api/drivers/amphora_driver/v2/driver.py
View File

@ -200,9 +200,7 @@ class AmphoraProviderDriver(driver_base.ProviderDriver):
consts.MEMBER_UPDATES: member_dict}
self.client.cast({}, 'update_member', **payload)
def member_batch_update(self, members):
# Get a list of existing members
pool_id = members[0].pool_id
def member_batch_update(self, pool_id, members):
# The DB should not have updated yet, so we can still use the pool
db_pool = self.repositories.pool.get(db_apis.get_session(), id=pool_id)
@ -235,10 +233,13 @@ class AmphoraProviderDriver(driver_base.ProviderDriver):
if m.id not in new_member_ids:
deleted_members.append(m)
payload = {'old_member_ids': [m.id for m in deleted_members],
'new_member_ids': [m.member_id for m in new_members],
'updated_members': updated_members}
self.client.cast({}, 'batch_update_members', **payload)
if deleted_members or new_members or updated_members:
payload = {'old_member_ids': [m.id for m in deleted_members],
'new_member_ids': [m.member_id for m in new_members],
'updated_members': updated_members}
self.client.cast({}, 'batch_update_members', **payload)
else:
LOG.info("Member batch update is a noop, returning early.")
def _validate_members(self, db_pool, members):
if db_pool.protocol == consts.PROTOCOL_UDP:

11
octavia/api/drivers/noop_driver/driver.py
View File

@ -148,10 +148,11 @@ class NoopManager(object):
self.driverconfig[new_member.member_id] = (
new_member, 'member_update')
def member_batch_update(self, members):
def member_batch_update(self, pool_id, members):
for member in members:
LOG.debug('Provider %s no-op, member_batch_update member %s',
self.__class__.__name__, member.member_id)
LOG.debug('Provider %s no-op, member_batch_update pool_id %s '
'member %s',
self.__class__.__name__, pool_id, member.member_id)
self.driverconfig[member.member_id] = (member,
'member_batch_update')
@ -294,8 +295,8 @@ class NoopProviderDriver(driver_base.ProviderDriver):
def member_update(self, old_member, new_member):
self.driver.member_update(old_member, new_member)
def member_batch_update(self, members):
self.driver.member_batch_update(members)
def member_batch_update(self, pool_id, members):
self.driver.member_batch_update(pool_id, members)
# Health Monitor
def health_monitor_create(self, healthmonitor):

3
octavia/api/v2/controllers/member.py
View File

@ -428,4 +428,5 @@ class MembersController(MemberController):
LOG.info("Sending Pool %s batch member update to provider %s",
db_pool.id, driver.name)
driver_utils.call_provider(
driver.name, driver.member_batch_update, provider_members)
driver.name, driver.member_batch_update, db_pool.id,
provider_members)

32
octavia/tests/functional/api/v2/test_member.py
View File

@ -643,7 +643,7 @@ class TestMember(base.BaseAPITest):
mock_provider.assert_called_once_with(u'noop_driver',
mock_driver.member_batch_update,
provider_creates)
self.pool_id, provider_creates)
@mock.patch('octavia.api.drivers.driver_factory.get_driver')
@mock.patch('octavia.api.drivers.utils.call_provider')
@ -686,7 +686,7 @@ class TestMember(base.BaseAPITest):
mock_provider.assert_called_once_with(u'noop_driver',
mock_driver.member_batch_update,
provider_members)
self.pool_id, provider_members)
def test_create_batch_members_with_bad_subnet(self):
subnet_id = uuidutils.generate_uuid()
@ -788,7 +788,7 @@ class TestMember(base.BaseAPITest):
mock_provider.assert_called_once_with(u'noop_driver',
mock_driver.member_batch_update,
provider_creates)
self.pool_id, provider_creates)
@mock.patch('octavia.api.drivers.driver_factory.get_driver')
@mock.patch('octavia.api.drivers.utils.call_provider')
@ -839,7 +839,7 @@ class TestMember(base.BaseAPITest):
mock_provider.assert_called_once_with(u'noop_driver',
mock_driver.member_batch_update,
provider_members)
self.pool_id, provider_members)
@mock.patch('octavia.api.drivers.driver_factory.get_driver')
@mock.patch('octavia.api.drivers.utils.call_provider')
@ -881,7 +881,29 @@ class TestMember(base.BaseAPITest):
mock_provider.assert_called_once_with(u'noop_driver',
mock_driver.member_batch_update,
provider_members)
self.pool_id, provider_members)
@mock.patch('octavia.api.drivers.driver_factory.get_driver')
@mock.patch('octavia.api.drivers.utils.call_provider')
def test_delete_batch_members_already_empty(self, mock_provider,
mock_get_driver):
mock_driver = mock.MagicMock()
mock_driver.name = 'noop_driver'
mock_get_driver.return_value = mock_driver
req_dict = []
body = {self.root_tag_list: req_dict}
path = self.MEMBERS_PATH.format(pool_id=self.pool_id)
self.put(path, body, status=202)
returned_members = self.get(
self.MEMBERS_PATH.format(pool_id=self.pool_id)
).json.get(self.root_tag_list)
self.assertEqual([], returned_members)
mock_provider.assert_called_once_with(u'noop_driver',
mock_driver.member_batch_update,
self.pool_id, [])
def test_create_with_attached_listener(self):
api_member = self.create_member(

24
octavia/tests/unit/api/drivers/amphora_driver/v1/test_amphora_driver.py
View File

@ -346,7 +346,8 @@ class TestAmphoraDriver(base.TestRpc):
'protocol_port': 80,
'pool_id': self.sample_data.pool1_id}
self.amp_driver.member_batch_update(prov_members)
self.amp_driver.member_batch_update(
self.sample_data.pool1_id, prov_members)
payload = {'old_member_ids': [self.sample_data.member1_id],
'new_member_ids': [self.sample_data.member3_id],
@ -380,13 +381,27 @@ class TestAmphoraDriver(base.TestRpc):
'protocol_port': 80,
'pool_id': self.sample_data.pool1_id}
self.amp_driver.member_batch_update(prov_members)
self.amp_driver.member_batch_update(
self.sample_data.pool1_id, prov_members)
payload = {'old_member_ids': [self.sample_data.member1_id],
'new_member_ids': [self.sample_data.member3_id],
'updated_members': [update_mem_dict]}
mock_cast.assert_called_with({}, 'batch_update_members', **payload)
@mock.patch('octavia.db.api.get_session')
@mock.patch('octavia.db.repositories.PoolRepository.get')
@mock.patch('oslo_messaging.RPCClient.cast')
def test_member_batch_update_clear_already_empty(
self, mock_cast, mock_pool_get, mock_session):
mock_pool = mock.MagicMock()
mock_pool_get.return_value = mock_pool
self.amp_driver.member_batch_update(
self.sample_data.pool1_id, [])
mock_cast.assert_not_called()
# Health Monitor
@mock.patch('oslo_messaging.RPCClient.cast')
def test_health_monitor_create(self, mock_cast):
@ -441,7 +456,8 @@ class TestAmphoraDriver(base.TestRpc):
'protocol_port': 80,
'pool_id': self.sample_data.pool1_id}
self.amp_driver.member_batch_update(prov_members)
self.amp_driver.member_batch_update(
self.sample_data.pool1_id, prov_members)
payload = {'old_member_ids': [self.sample_data.member1_id],
'new_member_ids': [self.sample_data.member3_id],
@ -479,7 +495,7 @@ class TestAmphoraDriver(base.TestRpc):
self.assertRaises(exceptions.UnsupportedOptionError,
self.amp_driver.member_batch_update,
prov_members)
self.sample_data.pool1_id, prov_members)
@mock.patch('oslo_messaging.RPCClient.cast')
def test_health_monitor_update(self, mock_cast):

24
octavia/tests/unit/api/drivers/amphora_driver/v2/test_amphora_driver.py
View File

@ -346,7 +346,8 @@ class TestAmphoraDriver(base.TestRpc):
'protocol_port': 80,
'pool_id': self.sample_data.pool1_id}
self.amp_driver.member_batch_update(prov_members)
self.amp_driver.member_batch_update(
self.sample_data.pool1_id, prov_members)
payload = {'old_member_ids': [self.sample_data.member1_id],
'new_member_ids': [self.sample_data.member3_id],
@ -380,13 +381,27 @@ class TestAmphoraDriver(base.TestRpc):
'protocol_port': 80,
'pool_id': self.sample_data.pool1_id}
self.amp_driver.member_batch_update(prov_members)
self.amp_driver.member_batch_update(
self.sample_data.pool1_id, prov_members)
payload = {'old_member_ids': [self.sample_data.member1_id],
'new_member_ids': [self.sample_data.member3_id],
'updated_members': [update_mem_dict]}
mock_cast.assert_called_with({}, 'batch_update_members', **payload)
@mock.patch('octavia.db.api.get_session')
@mock.patch('octavia.db.repositories.PoolRepository.get')
@mock.patch('oslo_messaging.RPCClient.cast')
def test_member_batch_update_clear_already_empty(
self, mock_cast, mock_pool_get, mock_session):
mock_pool = mock.MagicMock()
mock_pool_get.return_value = mock_pool
self.amp_driver.member_batch_update(
self.sample_data.pool1_id, [])
mock_cast.assert_not_called()
# Health Monitor
@mock.patch('oslo_messaging.RPCClient.cast')
def test_health_monitor_create(self, mock_cast):
@ -441,7 +456,8 @@ class TestAmphoraDriver(base.TestRpc):
'protocol_port': 80,
'pool_id': self.sample_data.pool1_id}
self.amp_driver.member_batch_update(prov_members)
self.amp_driver.member_batch_update(
self.sample_data.pool1_id, prov_members)
payload = {'old_member_ids': [self.sample_data.member1_id],
'new_member_ids': [self.sample_data.member3_id],
@ -479,7 +495,7 @@ class TestAmphoraDriver(base.TestRpc):
self.assertRaises(exceptions.UnsupportedOptionError,
self.amp_driver.member_batch_update,
prov_members)
self.sample_data.pool1_id, prov_members)
@mock.patch('oslo_messaging.RPCClient.cast')
def test_health_monitor_update(self, mock_cast):

2
octavia/tests/unit/api/drivers/test_provider_noop_driver.py
View File

@ -230,7 +230,7 @@ class TestNoopProviderDriver(base.TestCase):
self.driver.driver.driverconfig[self.member_id])
def test_member_batch_update(self):
self.driver.member_batch_update([self.ref_member])
self.driver.member_batch_update(self.pool_id, [self.ref_member])
self.assertEqual((self.ref_member, 'member_batch_update'),
self.driver.driver.driverconfig[self.member_id])