openstack
/
neutron
Code Issues Proposed changes

Update OVN client _get_port_options() code and utils 

The OVN client code in _get_port_options() was changed in
the following ways:

1) variable ip_subnets was changed to port_fixed_ips to
   reflect what it actually was.
2) Instead of just passing the list of subnet dictionaries
   to callers and having them iterate it, create a
   "subnets by id" dictionary that can be used in multiple
   places, including the OVN utilities.
3) Move the calls of get_subnets_address_scopes() and
   get_port_type_virtual_and_parents() so they are only
   made if there are subnets associated with the port.

The OVN utility code was changed to accept the "subnets by id"
dictionary mentioned in #2.

Functionally the code should be identical.

Required a lot of test cleanup.

Change-Id: I3dd4c283485c316df0662b5d679b6e13f65b4841
This commit is contained in:
Brian Haley 2023-05-31 17:31:50 -04:00
parent 04d982746e
commit 29505104aa
3 changed files with 62 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
neutron/common/ovn/utils.py
View File

@ -894,7 +894,7 @@ def get_ovn_chassis_other_config(chassis):
return chassis.external_ids
def get_subnets_address_scopes(context, subnets, fixed_ips, ml2_plugin):
def get_subnets_address_scopes(context, subnets_by_id, fixed_ips, ml2_plugin):
"""Returns the IPv4 and IPv6 address scopes of several subnets.
The subnets hosted on the same network must be allocated from the same
@ -903,17 +903,16 @@ def get_subnets_address_scopes(context, subnets, fixed_ips, ml2_plugin):
one for IPv4 and one for IPv6).
:param context: neutron api request context
:param subnets: (list of dict) subnet dictionaries
:param subnets_by_id: (dict) of subnets {subnet_id: subnet, ...}
:param fixed_ips: (list of dict) fixed IPs of several subnets (usually
belonging to a network but not mandatory)
:param ml2_plugin: (``Ml2Plugin``) ML2 plugin instance
:return: (tuple of 2 strings) IPv4 and IPv6 address scope IDs
"""
address4_scope_id, address6_scope_id = '', ''
if not subnets:
if not subnets_by_id:
return address4_scope_id, address6_scope_id
subnets_by_id = {subnet['id']: subnet for subnet in subnets}
for fixed_ip in fixed_ips:
subnet_id = fixed_ip.get('subnet_id')
subnet = subnets_by_id.get(subnet_id)
@ -1009,11 +1008,11 @@ def sync_ha_chassis_group(context, network_id, nb_idl, sb_idl, txn):
return ha_ch_grp.uuid if ha_ch_grp else hcg_cmd
def get_port_type_virtual_and_parents(subnets, fixed_ips, network_id, port_id,
nb_idl):
def get_port_type_virtual_and_parents(subnets_by_id, fixed_ips, network_id,
port_id, nb_idl):
"""Returns if a port is type virtual and its corresponding parents.
:param subnets: (list of dict) subnet dictionaries
:param subnets_by_id: (dict) of subnets {subnet_id: subnet, ...}
:param fixed_ips: (list of dict) fixed IPs of several subnets (usually
belonging to a network but not mandatory)
:param network_id: (string) network ID
@ -1023,10 +1022,9 @@ def get_port_type_virtual_and_parents(subnets, fixed_ips, network_id, port_id,
(2) the virtual IP address and (3) the virtual parents
"""
port_type, virtual_ip, virtual_parents = '', None, None
if not subnets:
if not subnets_by_id:
return port_type, virtual_ip, virtual_parents
subnets_by_id = {subnet['id'] for subnet in subnets}
for fixed_ip in fixed_ips:
if fixed_ip.get('subnet_id') not in subnets_by_id:
continue
@ -1109,8 +1107,9 @@ def validate_port_binding_and_virtual_port(
subnets = ml2_plugin.get_subnets(port_context.plugin_context,
filters={'id': list(subnet_ids)})
subnets_by_id = {subnet['id']: subnet for subnet in subnets}
port_type, _, _ = get_port_type_virtual_and_parents(
subnets, fixed_ips, port['network_id'], port['id'], nb_idl)
subnets_by_id, fixed_ips, port['network_id'], port['id'], nb_idl)
if port_type == constants.LSP_TYPE_VIRTUAL:
raise n_exc.BadRequest(
resource='port',

26
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
View File

@ -339,39 +339,35 @@ class OVNClient(object):
tag = bp_info.bp_param.get('tag', [])
address = port['mac_address']
ip_subnets = port.get('fixed_ips', [])
port_fixed_ips = port.get('fixed_ips', [])
subnet_ids = [
ip['subnet_id']
for ip in ip_subnets
for ip in port_fixed_ips
if 'subnet_id' in ip
]
subnets = self._plugin.get_subnets(
context, filters={'id': subnet_ids})
subnets_by_id = {subnet['id']: subnet for subnet in subnets}
address4_scope_id, address6_scope_id = (
utils.get_subnets_address_scopes(context, subnets, ip_subnets,
utils.get_subnets_address_scopes(context, subnets_by_id,
port_fixed_ips,
self._plugin))
p_type, virtual_ip, virtual_parents = (
utils.get_port_type_virtual_and_parents(
subnets, ip_subnets, port['network_id'], port['id'],
self._nb_idl))
subnets_by_id, port_fixed_ips, port['network_id'],
port['id'], self._nb_idl))
if p_type:
port_type = ovn_const.LSP_TYPE_VIRTUAL
options[ovn_const.LSP_OPTIONS_VIRTUAL_IP_KEY] = virtual_ip
options[ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY] = (
virtual_parents)
if subnets:
for ip in ip_subnets:
if subnets_by_id:
for ip in port_fixed_ips:
ip_addr = ip['ip_address']
address += ' ' + ip_addr
try:
subnet = [
sub
for sub in subnets
if sub["id"] == ip["subnet_id"]
][0]
except IndexError:
subnet = subnets_by_id.get(ip['subnet_id'])
if not subnet:
LOG.debug('Subnet not found for ip address %s',
ip_addr)
continue

84
neutron/tests/unit/common/ovn/test_utils.py
View File

@ -982,99 +982,99 @@ class GetSubnetsAddressScopeTestCase(base.BaseTestCase):
self.ml2_plugin = mock.Mock()
def test_no_subnets(self):
subnets = []
subnets_by_id = []
fixed_ips = mock.ANY
address4, address6 = utils.get_subnets_address_scopes(
mock.ANY, subnets, fixed_ips, self.ml2_plugin)
mock.ANY, subnets_by_id, fixed_ips, self.ml2_plugin)
self.assertEqual(('', ''), (address4, address6))
def test_no_subnetpool(self):
subnets = [
{'id': 'subnet1', 'subnetpool_id': None},
{'id': 'subnet2', 'subnetpool_id': None},
]
subnets_by_id = {
'subnet_id1': {'name': 'subnet1', 'subnetpool_id': None},
'subnet_id2': {'name': 'subnet2', 'subnetpool_id': None},
}
fixed_ips = [
{'subnet_id': 'subnet1'},
{'subnet_id': 'subnet2'},
{'subnet_id': 'subnet_id1'},
{'subnet_id': 'subnet_id2'},
]
address4, address6 = utils.get_subnets_address_scopes(
mock.ANY, subnets, fixed_ips, self.ml2_plugin)
mock.ANY, subnets_by_id, fixed_ips, self.ml2_plugin)
self.assertEqual(('', ''), (address4, address6))
def test_no_address_scope(self):
subnets = [
{'id': 'subnet1', 'subnetpool_id': 'pool_ipv4'},
{'id': 'subnet2', 'subnetpool_id': 'pool_ipv6'},
]
subnets_by_id = {
'subnet_id1': {'name': 'subnet1', 'subnetpool_id': 'pool_ipv4'},
'subnet_id2': {'name': 'subnet2', 'subnetpool_id': 'pool_ipv6'},
}
fixed_ips = [
{'subnet_id': 'subnet1'},
{'subnet_id': 'subnet2'},
{'subnet_id': 'subnet_id1'},
{'subnet_id': 'subnet_id2'},
]
self.ml2_plugin.get_subnetpool.side_effect = n_exc.SubnetPoolNotFound(
subnetpool_id='snp')
address4, address6 = utils.get_subnets_address_scopes(
mock.ANY, subnets, fixed_ips, self.ml2_plugin)
mock.ANY, subnets_by_id, fixed_ips, self.ml2_plugin)
self.assertEqual(('', ''), (address4, address6))
def test_address_scope(self):
subnets = [
{'id': 'subnet1', 'subnetpool_id': 'pool_ipv4'},
{'id': 'subnet2', 'subnetpool_id': 'pool_ipv6'},
]
subnets_by_id = {
'subnet_id1': {'name': 'subnet1', 'subnetpool_id': 'pool_ipv4'},
'subnet_id2': {'name': 'subnet2', 'subnetpool_id': 'pool_ipv6'},
}
fixed_ips = [
{'subnet_id': 'subnet1'},
{'subnet_id': 'subnet2'},
{'subnet_id': 'subnet_id1'},
{'subnet_id': 'subnet_id2'},
]
self.ml2_plugin.get_subnetpool.side_effect = [
{'address_scope_id': 'scope4', 'ip_version': n_const.IP_VERSION_4},
{'address_scope_id': 'scope6', 'ip_version': n_const.IP_VERSION_6},
]
address4, address6 = utils.get_subnets_address_scopes(
mock.ANY, subnets, fixed_ips, self.ml2_plugin)
mock.ANY, subnets_by_id, fixed_ips, self.ml2_plugin)
self.assertEqual(('scope4', 'scope6'), (address4, address6))
class GetPortTypeVirtualAndParentsTestCase(base.BaseTestCase):
def test_no_subnets(self):
subnets = []
subnets_by_id = []
fixed_ips = []
port_type, virtual_ip, virtual_parents = (
utils.get_port_type_virtual_and_parents(subnets, fixed_ips, 'net1',
'port1', mock.ANY))
utils.get_port_type_virtual_and_parents(subnets_by_id, fixed_ips,
'net1', 'port1', mock.ANY))
self.assertEqual(('', None, None),
(port_type, virtual_ip, virtual_parents))
@mock.patch.object(utils, 'get_virtual_port_parents', return_value=[])
def test_no_parents(self, *args):
subnets = [
{'id': 'subnet1'},
{'id': 'subnet2'},
]
subnets_by_id = {
'subnet_id1': {'name': 'subnet1'},
'subnet_id2': {'name': 'subnet2'},
}
fixed_ips = [
{'subnet_id': 'subnet1', 'ip_address': '1.2.3.4'},
{'subnet_id': 'subnet2', 'ip_address': '1.2.3.5'},
{'subnet_id': 'subnet_id1', 'ip_address': '1.2.3.4'},
{'subnet_id': 'subnet_id2', 'ip_address': '1.2.3.5'},
]
port_type, virtual_ip, virtual_parents = (
utils.get_port_type_virtual_and_parents(subnets, fixed_ips, 'net1',
'port1', mock.ANY))
utils.get_port_type_virtual_and_parents(subnets_by_id, fixed_ips,
'net1', 'port1', mock.ANY))
self.assertEqual(('', None, None),
(port_type, virtual_ip, virtual_parents))
@mock.patch.object(utils, 'get_virtual_port_parents',
return_value=['parent1', 'parent2'])
def test_with_parents(self, *args):
subnets = [
{'id': 'subnet1'},
{'id': 'subnet2'},
]
subnets_by_id = {
'subnet_id1': {'name': 'subnet1'},
'subnet_id2': {'name': 'subnet2'},
}
fixed_ips = [
{'subnet_id': 'subnet1', 'ip_address': '1.2.3.4'},
{'subnet_id': 'subnet2', 'ip_address': '1.2.3.5'},
{'subnet_id': 'subnet_id1', 'ip_address': '1.2.3.4'},
{'subnet_id': 'subnet_id2', 'ip_address': '1.2.3.5'},
]
port_type, virtual_ip, virtual_parents = (
utils.get_port_type_virtual_and_parents(subnets, fixed_ips, 'net1',
'port1', mock.ANY))
utils.get_port_type_virtual_and_parents(subnets_by_id, fixed_ips,
'net1', 'port1', mock.ANY))
self.assertEqual((constants.LSP_TYPE_VIRTUAL, '1.2.3.4',
'parent1,parent2'),
(port_type, virtual_ip, virtual_parents))