openstack
/
ovn-octavia-provider
Code Issues Proposed changes

Merge "Fix deletion of members without subnet_id"

This commit is contained in:
Zuul 2022-03-25 09:59:53 +00:00 committed by Gerrit Code Review
parent d6adbcef86 82eabfda24
commit 971057ae0b
4 changed files with 130 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
ovn_octavia_provider/driver.py
View File

@ -272,11 +272,20 @@ class OvnProviderDriver(driver_base.ProviderDriver):
self._ovn_helper.add_request(request)
def member_delete(self, member):
# NOTE(froyo): OVN provider allow to create member without param
# subnet_id, in that case the driver search it according to the
# pool_id, but it is not propagated to Octavia. In this case, if
# the member is deleted, Octavia send the object without subnet_id.
subnet_id = member.subnet_id
if (isinstance(subnet_id, o_datamodels.UnsetType) or not subnet_id):
subnet_id = self._ovn_helper._get_subnet_from_pool(
member.pool_id)
request_info = {'id': member.member_id,
'address': member.address,
'protocol_port': member.protocol_port,
'pool_id': member.pool_id,
'subnet_id': member.subnet_id}
'subnet_id': subnet_id}
request = {'type': ovn_const.REQ_TYPE_MEMBER_DELETE,
'info': request_info}
self._ovn_helper.add_request(request)
@ -286,13 +295,22 @@ class OvnProviderDriver(driver_base.ProviderDriver):
request_info = {'id': member.member_id,
'address': member.address,
'pool_id': member.pool_id,
'subnet_id': member.subnet_id,
'subnet_id': subnet_id,
'action': ovn_const.REQ_INFO_MEMBER_DELETED}
request = {'type': ovn_const.REQ_TYPE_HANDLE_MEMBER_DVR,
'info': request_info}
self._ovn_helper.add_request(request)
def member_update(self, old_member, new_member):
# NOTE(froyo): OVN provider allow to create member without param
# subnet_id, in that case the driver search it according to the
# pool_id, but it is not propagated to Octavia. In this case, if
# the member is updated, Octavia send the object without subnet_id.
subnet_id = old_member.subnet_id
if (isinstance(subnet_id, o_datamodels.UnsetType) or not subnet_id):
subnet_id = self._ovn_helper._get_subnet_from_pool(
old_member.pool_id)
# Validate monitoring options if present
self._check_member_monitor_options(new_member)
if new_member.address and self._ip_version_differs(new_member):
@ -301,7 +319,7 @@ class OvnProviderDriver(driver_base.ProviderDriver):
'address': old_member.address,
'protocol_port': old_member.protocol_port,
'pool_id': old_member.pool_id,
'subnet_id': old_member.subnet_id,
'subnet_id': subnet_id,
'old_admin_state_up': old_member.admin_state_up}
if not isinstance(new_member.admin_state_up, o_datamodels.UnsetType):
request_info['admin_state_up'] = new_member.admin_state_up
@ -317,19 +335,30 @@ class OvnProviderDriver(driver_base.ProviderDriver):
pool = external_ids[pool_key]
existing_members = pool.split(',') if pool else []
members_to_delete = copy.copy(existing_members)
pool_subnet_id = None
for member in members:
if (self._check_monitor_options(member) or
member.address and self._ip_version_differs(member)):
skipped_members.append(member.member_id)
continue
# NOTE(mjozefcz): We need to have subnet_id information.
if (isinstance(member.subnet_id, o_datamodels.UnsetType) or
not member.subnet_id):
msg = _('Subnet is required for Member creation '
'with OVN Provider Driver')
raise driver_exceptions.UnsupportedOptionError(
user_fault_string=msg,
operator_fault_string=msg)
# NOTE(froyo): if subnet_id not provided, lets try to get it
# from the member pool_id
subnet_id = member.subnet_id
if (isinstance(subnet_id, o_datamodels.UnsetType) or
not subnet_id):
pool_subnet_id = (
pool_subnet_id
if pool_subnet_id
else self._ovn_helper._get_subnet_from_pool(pool_id))
# NOTE(mjozefcz): We need to have subnet_id information.
if not pool_subnet_id:
msg = _('Subnet is required, or Loadbalancer associated '
'with Pool must have a subnet, for Member '
'creation with OVN Provider Driver')
raise driver_exceptions.UnsupportedOptionError(
user_fault_string=msg,
operator_fault_string=msg)
member.subnet_id = pool_subnet_id
admin_state_up = member.admin_state_up
if isinstance(admin_state_up, o_datamodels.UnsetType):
admin_state_up = True

25
ovn_octavia_provider/tests/functional/base.py
View File

@ -808,12 +808,21 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase,
if m.address == member_address:
return m
def _update_member_and_validate(self, lb_data, pool_id, member_address):
def _update_member_and_validate(self, lb_data, pool_id, member_address,
remove_subnet_id=False):
pool = self._get_pool_from_lb_data(lb_data, pool_id=pool_id)
member = self._get_pool_member(pool, member_address)
self._o_driver_lib.update_loadbalancer_status.reset_mock()
self.ovn_driver.member_update(member, member)
old_member = copy.deepcopy(member)
# NOTE(froyo): In order to test update of member without passing the
# subnet_id parameter of the member, just to cover the case when a new
# member has been created without passing that argument
if remove_subnet_id:
old_member.subnet_id = None
self.ovn_driver.member_update(old_member, member)
expected_status = {
'pools': [{'id': pool.pool_id,
'provisioning_status': 'ACTIVE'}],
@ -866,7 +875,7 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase,
check_call=False)
def _delete_member_and_validate(self, lb_data, pool_id, network_id,
member_address):
member_address, remove_subnet_id=False):
pool = self._get_pool_from_lb_data(lb_data, pool_id=pool_id)
member = self._get_pool_member(pool, member_address)
pool.members.remove(member)
@ -877,7 +886,15 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase,
pool_status['operating_status'] = o_constants.OFFLINE
self._o_driver_lib.update_loadbalancer_status.reset_mock()
self.ovn_driver.member_delete(member)
# NOTE(froyo): In order to test deletion of member without passing
# the subnet_id parameter of the member, just to cover the case when
# a new member has been created without passing that argument
m_member = copy.deepcopy(member)
if remove_subnet_id:
m_member.subnet_id = None
self.ovn_driver.member_delete(m_member)
expected_status = {
'pools': [pool_status],
'members': [{"id": member.member_id,

19
ovn_octavia_provider/tests/functional/test_driver.py
View File

@ -191,11 +191,28 @@ class TestOvnOctaviaProviderDriver(ovn_base.TestOvnOctaviaBase):
self._o_driver_lib.get_pool.return_value = lb_data['pools'][0]
self._o_driver_lib.get_loadbalancer.return_value = lb_data['model']
# Test creating Member without subnet but with pool
# Test deleting a member without subnet
self._create_member_and_validate(
lb_data, pool_TCP_id, None,
lb_data['vip_net_info'][0], '10.0.0.10',
expected_subnet=lb_data['vip_net_info'][1])
self._delete_member_and_validate(
lb_data, pool_TCP_id, lb_data['vip_net_info'][0],
'10.0.0.10', remove_subnet_id=True)
# Test update member without subnet
self._create_member_and_validate(
lb_data, pool_TCP_id, None,
lb_data['vip_net_info'][0], '10.0.0.10',
expected_subnet=lb_data['vip_net_info'][1])
self._update_member_and_validate(
lb_data, pool_TCP_id, "10.0.0.10", remove_subnet_id=True)
# Test creating a Member without subnet but with pool
self._create_member_and_validate(
lb_data, pool_TCP_id, None,
lb_data['vip_net_info'][0], '10.0.0.11',
expected_subnet=lb_data['vip_net_info'][1])
# Deleting the pool should also delete the members.
self._delete_pool_and_validate(lb_data, "p_TCP")

51
ovn_octavia_provider/tests/unit/test_driver.py
View File

@ -343,6 +343,23 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
self.driver.member_update(self.ref_member, self.update_member)
self.mock_add_request.assert_called_once_with(expected_dict)
def test_member_update_missing_subnet_id(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id)
info = {'id': self.update_member.member_id,
'address': self.ref_member.address,
'protocol_port': self.ref_member.protocol_port,
'pool_id': self.ref_member.pool_id,
'admin_state_up': self.update_member.admin_state_up,
'old_admin_state_up': self.ref_member.admin_state_up,
'subnet_id': self.ref_member.subnet_id}
expected_dict = {'type': ovn_const.REQ_TYPE_MEMBER_UPDATE,
'info': info}
member = copy.copy(self.ref_member)
member.subnet_id = data_models.UnsetType()
self.driver.member_update(member, self.update_member)
self.mock_add_request.assert_called_once_with(expected_dict)
@mock.patch.object(ovn_driver.OvnProviderDriver, '_ip_version_differs')
def test_member_update_no_ip_addr(self, mock_ip_differs):
self.update_member.address = None
@ -388,6 +405,12 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
self.driver.member_batch_update,
self.pool_id, [self.ref_member])
def test_member_batch_update_missing_subnet_id_get_from_pool(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id)
self.ref_member.subnet_id = None
self.driver.member_batch_update(self.pool_id, [self.ref_member])
def test_member_update_failure(self):
self.assertRaises(exceptions.UnsupportedOptionError,
self.driver.member_update, self.ref_member,
@ -422,6 +445,34 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
mock.call(expected_dict_dvr)]
self.mock_add_request.assert_has_calls(expected)
def test_member_delete_missing_subnet_id(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id)
info = {'id': self.ref_member.member_id,
'address': self.ref_member.address,
'protocol_port': self.ref_member.protocol_port,
'pool_id': self.ref_member.pool_id,
'subnet_id': self.ref_member.subnet_id}
expected_dict = {'type': ovn_const.REQ_TYPE_MEMBER_DELETE,
'info': info}
info_dvr = {
'id': self.ref_member.member_id,
'address': self.ref_member.address,
'pool_id': self.ref_member.pool_id,
'subnet_id': self.ref_member.subnet_id,
'action': ovn_const.REQ_INFO_MEMBER_DELETED}
expected_dict_dvr = {
'type': ovn_const.REQ_TYPE_HANDLE_MEMBER_DVR,
'info': info_dvr}
member = copy.copy(self.ref_member)
member.subnet_id = data_models.UnsetType()
self.driver.member_delete(member)
expected = [
mock.call(expected_dict),
mock.call(expected_dict_dvr)]
self.mock_add_request.assert_has_calls(expected)
def test_listener_create(self):
info = {'id': self.ref_listener.listener_id,
'protocol': self.ref_listener.protocol,