openstack
/
ovn-octavia-provider
Code Issues Proposed changes

Merge "Ensure members without subnet belong to VIP subnet or fail"

This commit is contained in:
Zuul 2022-07-21 15:44:35 +00:00 committed by Gerrit Code Review
parent 75e86cfa5e 136b829579
commit bb077b4eb7
5 changed files with 96 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
ovn_octavia_provider/driver.py
View File

@ -245,11 +245,15 @@ class OvnProviderDriver(driver_base.ProviderDriver):
admin_state_up = member.admin_state_up
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:
subnet_id, subnet_cidr = self._ovn_helper._get_subnet_from_pool(
member.pool_id)
if not (subnet_id and
self._ovn_helper._check_ip_in_subnet(member.address,
subnet_cidr)):
msg = _('Subnet is required, or Loadbalancer associated with '
'Pool must have a subnet, for Member creation '
'with OVN Provider Driver')
'with OVN Provider Driver if it is not the same as '
'LB VIP subnet')
raise driver_exceptions.UnsupportedOptionError(
user_fault_string=msg,
operator_fault_string=msg)
@ -285,8 +289,18 @@ class OvnProviderDriver(driver_base.ProviderDriver):
# 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(
subnet_id, subnet_cidr = self._ovn_helper._get_subnet_from_pool(
member.pool_id)
if not (subnet_id and
self._ovn_helper._check_ip_in_subnet(member.address,
subnet_cidr)):
msg = _('Subnet is required, or Loadbalancer associated with '
'Pool must have a subnet, for Member deletion if it is'
'with OVN Provider Driver if it is not the same as '
'LB VIP subnet')
raise driver_exceptions.UnsupportedOptionError(
user_fault_string=msg,
operator_fault_string=msg)
request_info = {'id': member.member_id,
'address': member.address,
@ -315,8 +329,18 @@ class OvnProviderDriver(driver_base.ProviderDriver):
# 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(
subnet_id, subnet_cidr = self._ovn_helper._get_subnet_from_pool(
old_member.pool_id)
if not (subnet_id and
self._ovn_helper._check_ip_in_subnet(new_member.address,
subnet_cidr)):
msg = _('Subnet is required, or Loadbalancer associated with '
'Pool must have a subnet, for Member update '
'with OVN Provider Driver if it is not the same as '
'LB VIP subnet')
raise driver_exceptions.UnsupportedOptionError(
user_fault_string=msg,
operator_fault_string=msg)
# Validate monitoring options if present
self._check_member_monitor_options(new_member)
@ -353,19 +377,23 @@ class OvnProviderDriver(driver_base.ProviderDriver):
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:
pool_subnet_id, pool_subnet_cidr = (
self._ovn_helper._get_subnet_from_pool(pool_id))
if pool_subnet_id:
if (self._ovn_helper._check_ip_in_subnet(
member.address, pool_subnet_cidr)):
member.subnet_id = pool_subnet_id
# NOTE(mjozefcz): We need to have subnet_id information.
if not member.subnet_id:
msg = _('Subnet is required, or Loadbalancer associated '
'with Pool must have a subnet, for Member '
'creation with OVN Provider Driver')
'batch update with OVN Provider Driver if it is '
'not the same as LB VIP subnet')
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

16
ovn_octavia_provider/helper.py
View File

@ -490,13 +490,25 @@ class OvnProviderHelper():
ovn_lb = self._find_ovn_lb_with_pool_key(pool_key)
return pool_key, ovn_lb
def _check_ip_in_subnet(self, ip, subnet):
return (netaddr.IPAddress(ip) in netaddr.IPNetwork(subnet))
def _get_subnet_from_pool(self, pool_id):
pool = self._octavia_driver_lib.get_pool(pool_id)
if not pool:
return
return None, None
lb = self._octavia_driver_lib.get_loadbalancer(pool.loadbalancer_id)
if lb and lb.vip_subnet_id:
return lb.vip_subnet_id
neutron_client = clients.get_neutron_client()
try:
subnet = neutron_client.show_subnet(lb.vip_subnet_id)
vip_subnet_cidr = subnet['subnet']['cidr']
except n_exc.NotFound:
LOG.warning('Subnet %s not found while trying to '
'fetch its data.', lb.vip_subnet_id)
return None, None
return lb.vip_subnet_id, vip_subnet_cidr
return None, None
def _execute_commands(self, commands):
with self.ovn_nbdb_api.transaction(check_error=True) as txn:

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

@ -62,12 +62,14 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase,
self.fake_neutron_client.show_port = self._mock_show_port
self.fake_neutron_client.delete_port.return_value = True
self._local_net_cache = {}
self._local_cidr_cache = {}
self._local_port_cache = {'ports': []}
self.core_plugin = directory.get_plugin()
def _mock_show_subnet(self, subnet_id):
subnet = {}
subnet['network_id'] = self._local_net_cache[subnet_id]
subnet['cidr'] = self._local_cidr_cache[subnet_id]
return {'subnet': subnet}
def _mock_list_ports(self, **kwargs):
@ -253,6 +255,7 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase,
cidr)
subnet = self.deserialize(self.fmt, res)['subnet']
self._local_net_cache[subnet['id']] = n1['network']['id']
self._local_cidr_cache[subnet['id']] = subnet['cidr']
port = self._make_port(self.fmt, n1['network']['id'])
if router_id:

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

@ -227,7 +227,11 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
self.mock_get_subnet_from_pool = mock.patch.object(
ovn_helper.OvnProviderHelper,
'_get_subnet_from_pool',
return_value=None).start()
return_value=(None, None)).start()
self.mock_check_ip_in_subnet = mock.patch.object(
ovn_helper.OvnProviderHelper,
'_check_ip_in_subnet',
return_value=True).start()
def test_check_for_allowed_cidrs_exception(self):
self.assertRaises(exceptions.UnsupportedOptionError,
@ -301,7 +305,18 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
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)
self.ref_member.subnet_id, '198.52.100.0/24')
self.driver._ovn_helper._check_ip_in_subnet.return_value = False
self.ref_member.subnet_id = data_models.UnsetType()
self.assertRaises(exceptions.UnsupportedOptionError,
self.driver.member_create, self.ref_member)
self.ref_member.subnet_id = None
self.assertRaises(exceptions.UnsupportedOptionError,
self.driver.member_create, self.ref_member)
def test_member_create_no_subnet_provided_get_from_pool_failed(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id, '198.52.100.0/24')
member = copy.copy(self.ref_member)
member.subnet_id = data_models.UnsetType()
self._test_member_create(member)
@ -349,7 +364,7 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
def test_member_update_missing_subnet_id(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id)
self.ref_member.subnet_id, '198.52.100.0/24')
info = {'id': self.update_member.member_id,
'address': self.ref_member.address,
'protocol_port': self.ref_member.protocol_port,
@ -366,7 +381,7 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
def test_member_update_unset_admin_state_up(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id)
self.ref_member.subnet_id, '198.52.100.0/24')
self.update_member.admin_state_up = data_models.UnsetType()
info = {'id': self.update_member.member_id,
'address': self.ref_member.address,
@ -428,10 +443,19 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
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, '198.52.100.0/24')
self.ref_member.subnet_id = None
self.driver.member_batch_update(self.pool_id, [self.ref_member])
def test_member_batch_update_missing_subnet_id_get_from_pool_fail(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id, '198.52.100.0/24')
self.driver._ovn_helper._check_ip_in_subnet.return_value = False
self.ref_member.subnet_id = None
self.assertRaises(exceptions.UnsupportedOptionError,
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,
@ -468,7 +492,7 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
def test_member_delete_missing_subnet_id(self):
self.driver._ovn_helper._get_subnet_from_pool.return_value = (
self.ref_member.subnet_id)
self.ref_member.subnet_id, '198.52.100.0/24')
info = {'id': self.ref_member.member_id,
'address': self.ref_member.address,
'protocol_port': self.ref_member.protocol_port,

14
ovn_octavia_provider/tests/unit/test_helper.py
View File

@ -376,7 +376,11 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
found = f(self.ovn_lb.id, protocol='tcp')
self.assertEqual(found, self.ovn_lb)
def test__get_subnet_from_pool(self):
@mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
def test__get_subnet_from_pool(self, net_cli):
net_cli.return_value.show_subnet.return_value = {
'subnet': {'cidr': '10.22.33.0/24'}}
f = self.helper._get_subnet_from_pool
lb = data_models.LoadBalancer(
@ -395,12 +399,12 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
with mock.patch.object(self.helper, '_octavia_driver_lib') as dlib:
dlib.get_pool.return_value = None
found = f('not_found')
self.assertIsNone(found)
self.assertEqual((None, None), 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)
self.assertEqual(found, (lb.vip_subnet_id, '10.22.33.0/24'))
def test__get_subnet_from_pool_lb_no_vip_subnet_id(self):
f = self.helper._get_subnet_from_pool
@ -420,12 +424,12 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
with mock.patch.object(self.helper, '_octavia_driver_lib') as dlib:
dlib.get_pool.return_value = None
found = f('not_found')
self.assertIsNone(found)
self.assertEqual((None, None), found)
dlib.get_pool.return_value = lb_pool
dlib.get_loadbalancer.return_value = lb
found = f(self.pool_id)
self.assertIsNone(found)
self.assertEqual((None, None), found)
def test__get_or_create_ovn_lb_no_lb_found(self):
self.mock_find_ovn_lbs.stop()