openstack
/
ovn-octavia-provider
Code Issues Proposed changes

Support creating members without a subnet ID 

Since subnet ID is an optional API argument, if not
available the provider driver will now attempt to look
it up via the pool ID that is required.

NOTE: Manually cherry-picked as there was a code reshape,
and in the backport all the code goes to driver.py

Closes-bug: #1896677
(manually cherry picked from commit 675ea9c35e)
Change-Id: I69a052aadf299907bf8ba1634376504461e2d779
This commit is contained in:
Luis Tomas Bolivar 2022-01-31 11:41:15 +01:00
parent 6885cfb046
commit bf86846205
4 changed files with 115 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
ovn_octavia_provider/driver.py
View File

@ -601,6 +601,14 @@ class OvnProviderHelper(object):
ovn_lb = self._find_ovn_lb_with_pool_key(pool_key)
return pool_key, ovn_lb
def _get_subnet_from_pool(self, pool_id):
pool = self._octavia_driver_lib.get_pool(pool_id)
if not pool:
return
lb = self._octavia_driver_lib.get_loadbalancer(pool.loadbalancer_id)
if lb and lb.vip_subnet_id:
return lb.vip_subnet_id
def _execute_commands(self, commands):
with self.ovn_nbdb_api.transaction(check_error=True) as txn:
for command in commands:
@ -2131,6 +2139,8 @@ class OvnProviderDriver(driver_base.ProviderDriver):
def _ip_version_differs(self, member):
_, ovn_lb = self._ovn_helper._find_ovn_lb_by_pool_id(member.pool_id)
if not ovn_lb:
return False
lb_vip = ovn_lb.external_ids[ovn_const.LB_EXT_IDS_VIP_KEY]
return netaddr.IPNetwork(lb_vip).version != (
netaddr.IPNetwork(member.address).version)
@ -2144,13 +2154,16 @@ class OvnProviderDriver(driver_base.ProviderDriver):
if self._ip_version_differs(member):
raise IPVersionsMixingNotSupportedError()
admin_state_up = member.admin_state_up
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)
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)
if not 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)
if isinstance(admin_state_up, o_datamodels.UnsetType):
admin_state_up = True
@ -2158,7 +2171,7 @@ class OvnProviderDriver(driver_base.ProviderDriver):
'address': member.address,
'protocol_port': member.protocol_port,
'pool_id': member.pool_id,
'subnet_id': member.subnet_id,
'subnet_id': subnet_id,
'admin_state_up': admin_state_up}
request = {'type': REQ_TYPE_MEMBER_CREATE,
'info': request_info}
@ -2170,7 +2183,7 @@ 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': REQ_INFO_MEMBER_ADDED}
request = {'type': REQ_TYPE_HANDLE_MEMBER_DVR,
'info': request_info}

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

@ -100,7 +100,7 @@ class TestOvnOctaviaBase(
return e1, e1_s1
def _create_lb_model(self, vip=None, vip_network_id=None,
vip_port_id=None,
vip_subnet_id=None, vip_port_id=None,
admin_state_up=True):
lb = octavia_data_model.LoadBalancer()
lb.loadbalancer_id = uuidutils.generate_uuid()
@ -112,6 +112,8 @@ class TestOvnOctaviaBase(
if vip_network_id:
lb.vip_network_id = vip_network_id
if vip_subnet_id:
lb.vip_subnet_id = vip_subnet_id
if vip_port_id:
lb.vip_port_id = vip_port_id
lb.admin_state_up = admin_state_up
@ -335,6 +337,7 @@ class TestOvnOctaviaBase(
lb_data['vip_net_info'] = net_info
lb_data['model'] = self._create_lb_model(vip=net_info[2],
vip_network_id=net_info[0],
vip_subnet_id=net_info[1],
vip_port_id=net_info[3],
admin_state_up=admin_state_up)
lb_data[ovn_const.LB_EXT_IDS_LS_REFS_KEY] = {}
@ -690,7 +693,7 @@ class TestOvnOctaviaBase(
return listeners
def _create_member_and_validate(self, lb_data, pool_id, subnet_id,
network_id, address):
network_id, address, expected_subnet=None):
self._o_driver_lib.update_loadbalancer_status.reset_mock()
pool = self._get_pool_from_lb_data(lb_data, pool_id=pool_id)
pool_status = {'id': pool.pool_id,
@ -698,7 +701,15 @@ class TestOvnOctaviaBase(
'operating_status': o_constants.ONLINE}
m_member = self._create_member_model(pool.pool_id, subnet_id, address)
pool.members.append(m_member)
# The "expected" member value, which might be different from what
# we pass to member_create(), for example, if an expected_subnet
# was given.
if expected_subnet:
e_member = copy.deepcopy(m_member)
e_member.subnet_id = expected_subnet
else:
e_member = m_member
pool.members.append(e_member)
self.ovn_driver.member_create(m_member)
self._update_ls_refs(lb_data, network_id)
@ -1006,13 +1017,39 @@ class TestOvnOctaviaProviderDriver(
'30.0.0.6')
self._delete_member_and_validate(lb_data, pool_1_id, net20, '20.0.0.6')
# Test creating Member without subnet
m_member = self._create_member_model(pool_1_id,
# Deleting the pool should also delete the members.
self._delete_pool_and_validate(lb_data, "p1")
# Delete the whole LB.
self._delete_load_balancer_and_validate(lb_data)
def test_member_no_subnet(self):
self._o_driver_lib.get_pool.return_value = None
# Test creating Member without subnet and unknown pool
m_member = self._create_member_model('pool_from_nowhere',
None,
'30.0.0.7', 80)
self.assertRaises(o_exceptions.UnsupportedOptionError,
self.ovn_driver.member_create, m_member)
lb_data = self._create_load_balancer_and_validate(
{'vip_network': 'vip_network',
'cidr': '10.0.0.0/24'})
# TCP Pool
self._create_pool_and_validate(lb_data, "p1", protocol='TCP')
pool_TCP_id = lb_data['pools'][0].pool_id
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
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])
# Deleting the pool should also delete the members.
self._delete_pool_and_validate(lb_data, "p1")

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

@ -279,6 +279,10 @@ class TestOvnProviderDriver(TestOvnOctaviaBase):
ovn_driver.OvnProviderHelper,
'_find_ovn_lb_with_pool_key',
return_value=self.ovn_lb).start()
self.mock_get_subnet_from_pool = mock.patch.object(
ovn_driver.OvnProviderHelper,
'_get_subnet_from_pool',
return_value=None).start()
def test__ip_version_differs(self):
self.assertFalse(self.driver._ip_version_differs(self.ref_member))
@ -292,7 +296,7 @@ class TestOvnProviderDriver(TestOvnOctaviaBase):
mock.call('pool_%s' % self.pool_id),
mock.call('pool_%s:D' % self.pool_id)])
def test_member_create(self):
def _test_member_create(self, member):
info = {'id': self.ref_member.member_id,
'address': self.ref_member.address,
'protocol_port': self.ref_member.protocol_port,
@ -310,12 +314,15 @@ class TestOvnProviderDriver(TestOvnOctaviaBase):
expected_dict_dvr = {
'type': ovn_driver.REQ_TYPE_HANDLE_MEMBER_DVR,
'info': info_dvr}
self.driver.member_create(self.ref_member)
self.driver.member_create(member)
expected = [
mock.call(expected_dict),
mock.call(expected_dict_dvr)]
self.mock_add_request.assert_has_calls(expected)
def test_member_create(self):
self._test_member_create(self.ref_member)
def test_member_create_failure(self):
self.assertRaises(exceptions.UnsupportedOptionError,
self.driver.member_create, self.fail_member)
@ -343,6 +350,15 @@ class TestOvnProviderDriver(TestOvnOctaviaBase):
self.assertRaises(exceptions.UnsupportedOptionError,
self.driver.member_create, self.ref_member)
def test_member_create_no_subnet_provided_get_from_pool(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id)
member = copy.copy(self.ref_member)
member.subnet_id = data_models.UnsetType()
self._test_member_create(member)
member.subnet_id = None
self._test_member_create(member)
def test_member_create_monitor_opts(self):
self.ref_member.monitor_address = '172.20.20.1'
self.assertRaises(exceptions.UnsupportedOptionError,
@ -871,6 +887,32 @@ class TestOvnProviderHelper(TestOvnOctaviaBase):
found = f(self.ovn_lb.id)
self.assertListEqual(found, [self.ovn_lb, udp_lb])
def test__get_subnet_from_pool(self):
f = self.helper._get_subnet_from_pool
lb = data_models.LoadBalancer(
loadbalancer_id=self.loadbalancer_id,
name='The LB',
vip_address=self.vip_address,
vip_subnet_id=self.vip_subnet_id,
vip_network_id=self.vip_network_id)
lb_pool = data_models.Pool(
loadbalancer_id=self.loadbalancer_id,
name='The pool',
pool_id=self.pool_id,
protocol='TCP')
with mock.patch.object(self.helper, '_octavia_driver_lib') as dlib:
dlib.get_pool.return_value = None
found = f('not_found')
self.assertIsNone(found)
dlib.get_pool.return_value = lb_pool
dlib.get_loadbalancer.return_value = lb
found = f(self.pool_id)
self.assertEqual(found, lb.vip_subnet_id)
def test__get_or_create_ovn_lb_no_lb_found(self):
self.mock_find_ovn_lbs.stop()
self.helper.ovn_nbdb_api.db_find_rows.return_value.\

7
releasenotes/notes/support-member-create-without-subnetid-0b4e3aa6ac453f28.yaml Normal file
View File

@ -0,0 +1,7 @@
---
fixes:
- |
Creating members without specifying a subnet ID is now supported.
Since the subnet ID is an optional API argument, if not given
the provider driver will now attempt to look it up via the pool
ID that is a required argument.