Add support for OVN LB selection fields
Prior this patch OVN Octavia provider driver used
default 5-tuple-hash algorithm which is pretty similar to
SOURCE_IP_PORT.
Unfornutelly because of the bug described here [1] it
was not clear how 5-tuple-hash works and some inconsistencies
between kernel and user space implementations were found.
OVN recently added support for selective fields in OVN LB, to
explicitly define what fields are being hashed to tackle this problem.
This commit adds support for that kind of hashing. If installation
of OVN on which OVN Octavia provider is running doesn't support
selective fields - it will use old behavior.
[1] https://mail.openvswitch.org/pipermail/ovs-discuss/2020-April/049896.html
[2] 5af304e747
Change-Id: I7b4ab99d1be2855e18b186557990c85f170ad548
Closes-Bug: #1871239
This commit is contained in:
Maciej Józefczyk
parent
6a811c4c3f
commit
23d743a444
@ -81,3 +81,11 @@ EXCEPTION_MSG = "Exception occurred during %s"
|
||||
|
||||
# Used in functional tests
|
||||
LR_REF_KEY_HEADER = 'neutron-'
|
||||
|
||||
# LB selection fields to represent LB algorithm
|
||||
LB_SELECTION_FIELDS_MAP = {
|
||||
constants.LB_ALGORITHM_SOURCE_IP_PORT: ["ip_dst", "ip_src",
|
||||
"tp_dst", "tp_src"],
|
||||
constants.LB_ALGORITHM_SOURCE_IP: ["ip_src", "ip_dst"],
|
||||
None: ["ip_src", "ip_dst", "tp_src", "tp_dst"],
|
||||
}
|
||||
|
||||
@ -103,6 +103,7 @@ class OvnProviderDriver(driver_base.ProviderDriver):
|
||||
request_info = {'id': pool.pool_id,
|
||||
'loadbalancer_id': pool.loadbalancer_id,
|
||||
'protocol': pool.protocol,
|
||||
'lb_algorithm': pool.lb_algorithm,
|
||||
'listener_id': pool.listener_id,
|
||||
'admin_state_up': admin_state_up}
|
||||
request = {'type': ovn_const.REQ_TYPE_POOL_CREATE,
|
||||
|
||||
@ -363,12 +363,17 @@ class OvnProviderHelper(object):
|
||||
raise idlutils.RowNotFound(table='Load_Balancer',
|
||||
col='name', match=lb_id)
|
||||
|
||||
def _get_or_create_ovn_lb(self, lb_id, protocol, admin_state_up):
|
||||
def _get_or_create_ovn_lb(
|
||||
self, lb_id, protocol, admin_state_up,
|
||||
lb_algorithm=constants.LB_ALGORITHM_SOURCE_IP_PORT):
|
||||
"""Find or create ovn lb with given protocol
|
||||
|
||||
Find the loadbalancer configured with given protocol or
|
||||
create required if not found
|
||||
"""
|
||||
# TODO(mjozefcz): For now we support only one LB algorithm.
|
||||
# As we may extend that in the future we would need to
|
||||
# look here also for lb_algorithm, along with protocol.
|
||||
# Make sure that its lowercase - OVN NBDB stores lowercases
|
||||
# for this field.
|
||||
protocol = protocol.lower()
|
||||
@ -395,6 +400,7 @@ class OvnProviderHelper(object):
|
||||
lb_info = {
|
||||
'id': lb_id,
|
||||
'protocol': protocol,
|
||||
constants.LB_ALGORITHM: lb_algorithm,
|
||||
'vip_address': ovn_lbs[0].external_ids.get(
|
||||
ovn_const.LB_EXT_IDS_VIP_KEY),
|
||||
'vip_port_id':
|
||||
@ -735,6 +741,14 @@ class OvnProviderHelper(object):
|
||||
return True
|
||||
return False
|
||||
|
||||
def _are_selection_fields_supported(self):
|
||||
return self.ovn_nbdb_api.is_col_present(
|
||||
'Load_Balancer', 'selection_fields')
|
||||
|
||||
@staticmethod
|
||||
def _get_selection_keys(lb_algorithm):
|
||||
return ovn_const.LB_SELECTION_FIELDS_MAP[lb_algorithm]
|
||||
|
||||
def check_lb_protocol(self, lb_id, listener_protocol):
|
||||
ovn_lb = self._find_ovn_lbs(lb_id, protocol=listener_protocol)
|
||||
if not ovn_lb:
|
||||
@ -778,12 +792,18 @@ class OvnProviderHelper(object):
|
||||
lr_ref = loadbalancer.get(ovn_const.LB_EXT_IDS_LR_REF_KEY)
|
||||
if lr_ref:
|
||||
external_ids[ovn_const.LB_EXT_IDS_LR_REF_KEY] = lr_ref
|
||||
|
||||
# In case we have LB algoritm set
|
||||
lb_algorithm = loadbalancer.get(constants.LB_ALGORITHM)
|
||||
kwargs = {
|
||||
'name': loadbalancer[constants.ID],
|
||||
'protocol': protocol,
|
||||
'external_ids': external_ids}
|
||||
if self._are_selection_fields_supported():
|
||||
kwargs['selection_fields'] = self._get_selection_keys(lb_algorithm)
|
||||
try:
|
||||
self.ovn_nbdb_api.db_create(
|
||||
'Load_Balancer', name=loadbalancer[constants.ID],
|
||||
protocol=protocol,
|
||||
external_ids=external_ids).execute(check_error=True)
|
||||
'Load_Balancer',
|
||||
**kwargs).execute(check_error=True)
|
||||
ovn_lb = self._find_ovn_lbs(
|
||||
loadbalancer[constants.ID],
|
||||
protocol=protocol)
|
||||
@ -1208,7 +1228,9 @@ class OvnProviderHelper(object):
|
||||
ovn_lb = self._get_or_create_ovn_lb(
|
||||
pool[constants.LOADBALANCER_ID],
|
||||
pool[constants.PROTOCOL],
|
||||
pool[constants.ADMIN_STATE_UP])
|
||||
pool[constants.ADMIN_STATE_UP],
|
||||
lb_algorithm=pool[constants.LB_ALGORITHM])
|
||||
|
||||
external_ids = copy.deepcopy(ovn_lb.external_ids)
|
||||
pool_key = self._get_pool_key(
|
||||
pool[constants.ID], is_enabled=pool[constants.ADMIN_STATE_UP])
|
||||
|
||||
@ -172,9 +172,13 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase,
|
||||
external_ids[
|
||||
ovn_const.LB_EXT_IDS_LS_REFS_KEY] = jsonutils.loads(
|
||||
ls_refs)
|
||||
lbs.append({'name': lb.name, 'protocol': lb.protocol,
|
||||
'vips': lb.vips, 'external_ids': external_ids})
|
||||
|
||||
lb_dict = {'name': lb.name, 'protocol': lb.protocol,
|
||||
'vips': lb.vips, 'external_ids': external_ids}
|
||||
try:
|
||||
lb_dict['selection_fields'] = lb.selection_fields
|
||||
except AttributeError:
|
||||
pass
|
||||
lbs.append(lb_dict)
|
||||
return lbs
|
||||
|
||||
def _get_loadbalancer_id(self, lb_name):
|
||||
@ -432,6 +436,12 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase,
|
||||
ovn_const.LR_REF_KEY_HEADER + vip_net_id))
|
||||
|
||||
def _make_expected_lbs(self, lb_data):
|
||||
def _get_lb_field_by_protocol(protocol, field='external_ids'):
|
||||
"Get needed external_ids and pass by reference"
|
||||
lb = [lb for lb in expected_lbs
|
||||
if lb.get('protocol') == [protocol]]
|
||||
return lb[0].get(field)
|
||||
|
||||
if not lb_data or not lb_data.get('model'):
|
||||
return []
|
||||
|
||||
@ -454,17 +464,16 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase,
|
||||
if len(expected_protocols) == 0:
|
||||
expected_protocols.append(None)
|
||||
|
||||
expected_lbs = [{'name': lb_data['model'].loadbalancer_id,
|
||||
'protocol': [protocol] if protocol else [],
|
||||
'vips': {},
|
||||
'external_ids': copy.deepcopy(external_ids)}
|
||||
for protocol in expected_protocols]
|
||||
|
||||
def _get_lb_field_by_protocol(protocol, field='external_ids'):
|
||||
"Get needed external_ids and pass by reference"
|
||||
lb = [lb for lb in expected_lbs
|
||||
if lb.get('protocol') == [protocol]]
|
||||
return lb[0].get(field)
|
||||
expected_lbs = []
|
||||
for protocol in expected_protocols:
|
||||
lb = {'name': lb_data['model'].loadbalancer_id,
|
||||
'protocol': [protocol] if protocol else [],
|
||||
'vips': {},
|
||||
'external_ids': copy.deepcopy(external_ids)}
|
||||
if self.ovn_driver._ovn_helper._are_selection_fields_supported():
|
||||
lb['selection_fields'] = ovn_const.LB_SELECTION_FIELDS_MAP[
|
||||
o_constants.LB_ALGORITHM_SOURCE_IP_PORT]
|
||||
expected_lbs.append(lb)
|
||||
|
||||
# For every connected subnet to the LB set the ref
|
||||
# counter.
|
||||
|
||||
@ -1,10 +1,11 @@
|
||||
{
|
||||
"name": "OVN_Northbound",
|
||||
"version": "5.16.0",
|
||||
"cksum": "923459061 23095",
|
||||
"version": "5.23.0",
|
||||
"cksum": "111023208 25806",
|
||||
"tables": {
|
||||
"NB_Global": {
|
||||
"columns": {
|
||||
"name": {"type": "string"},
|
||||
"nb_cfg": {"type": {"key": "integer"}},
|
||||
"sb_cfg": {"type": {"key": "integer"}},
|
||||
"hv_cfg": {"type": {"key": "integer"}},
|
||||
@ -59,7 +60,12 @@
|
||||
"min": 0, "max": "unlimited"}},
|
||||
"external_ids": {
|
||||
"type": {"key": "string", "value": "string",
|
||||
"min": 0, "max": "unlimited"}}},
|
||||
"min": 0, "max": "unlimited"}},
|
||||
"forwarding_groups": {
|
||||
"type": {"key": {"type": "uuid",
|
||||
"refTable": "Forwarding_Group",
|
||||
"refType": "strong"},
|
||||
"min": 0, "max": "unlimited"}}},
|
||||
"isRoot": true},
|
||||
"Logical_Switch_Port": {
|
||||
"columns": {
|
||||
@ -113,6 +119,18 @@
|
||||
"min": 0, "max": "unlimited"}}},
|
||||
"indexes": [["name"]],
|
||||
"isRoot": false},
|
||||
"Forwarding_Group": {
|
||||
"columns": {
|
||||
"name": {"type": "string"},
|
||||
"vip": {"type": "string"},
|
||||
"vmac": {"type": "string"},
|
||||
"liveness": {"type": "boolean"},
|
||||
"external_ids": {
|
||||
"type": {"key": "string", "value": "string",
|
||||
"min": 0, "max": "unlimited"}},
|
||||
"child_port": {"type": {"key": "string",
|
||||
"min": 1, "max": "unlimited"}}},
|
||||
"isRoot": false},
|
||||
"Address_Set": {
|
||||
"columns": {
|
||||
"name": {"type": "string"},
|
||||
@ -150,12 +168,39 @@
|
||||
"min": 0, "max": "unlimited"}},
|
||||
"protocol": {
|
||||
"type": {"key": {"type": "string",
|
||||
"enum": ["set", ["tcp", "udp"]]},
|
||||
"enum": ["set", ["tcp", "udp", "sctp"]]},
|
||||
"min": 0, "max": 1}},
|
||||
"health_check": {"type": {
|
||||
"key": {"type": "uuid",
|
||||
"refTable": "Load_Balancer_Health_Check",
|
||||
"refType": "strong"},
|
||||
"min": 0,
|
||||
"max": "unlimited"}},
|
||||
"ip_port_mappings": {
|
||||
"type": {"key": "string", "value": "string",
|
||||
"min": 0, "max": "unlimited"}},
|
||||
"selection_fields": {
|
||||
"type": {"key": {"type": "string",
|
||||
"enum": ["set",
|
||||
["eth_src", "eth_dst", "ip_src", "ip_dst",
|
||||
"tp_src", "tp_dst"]]},
|
||||
"min": 0, "max": "unlimited"}},
|
||||
"external_ids": {
|
||||
"type": {"key": "string", "value": "string",
|
||||
"min": 0, "max": "unlimited"}}},
|
||||
"isRoot": true},
|
||||
"Load_Balancer_Health_Check": {
|
||||
"columns": {
|
||||
"vip": {"type": "string"},
|
||||
"options": {
|
||||
"type": {"key": "string",
|
||||
"value": "string",
|
||||
"min": 0,
|
||||
"max": "unlimited"}},
|
||||
"external_ids": {
|
||||
"type": {"key": "string", "value": "string",
|
||||
"min": 0, "max": "unlimited"}}},
|
||||
"isRoot": false},
|
||||
"ACL": {
|
||||
"columns": {
|
||||
"name": {"type": {"key": {"type": "string",
|
||||
@ -303,6 +348,9 @@
|
||||
"ipv6_ra_configs": {
|
||||
"type": {"key": "string", "value": "string",
|
||||
"min": 0, "max": "unlimited"}},
|
||||
"ipv6_prefix": {"type": {"key": "string",
|
||||
"min": 0,
|
||||
"max": "unlimited"}},
|
||||
"external_ids": {
|
||||
"type": {"key": "string", "value": "string",
|
||||
"min": 0, "max": "unlimited"}}},
|
||||
@ -330,13 +378,17 @@
|
||||
"action": {"type": {
|
||||
"key": {"type": "string",
|
||||
"enum": ["set", ["allow", "drop", "reroute"]]}}},
|
||||
"nexthop": {"type": {"key": "string", "min": 0, "max": 1}}},
|
||||
"nexthop": {"type": {"key": "string", "min": 0, "max": 1}},
|
||||
"external_ids": {
|
||||
"type": {"key": "string", "value": "string",
|
||||
"min": 0, "max": "unlimited"}}},
|
||||
"isRoot": false},
|
||||
"NAT": {
|
||||
"columns": {
|
||||
"external_ip": {"type": "string"},
|
||||
"external_mac": {"type": {"key": "string",
|
||||
"min": 0, "max": 1}},
|
||||
"external_port_range": {"type": "string"},
|
||||
"logical_ip": {"type": "string"},
|
||||
"logical_port": {"type": {"key": "string",
|
||||
"min": 0, "max": 1}},
|
||||
@ -345,6 +397,8 @@
|
||||
"snat",
|
||||
"dnat_and_snat"
|
||||
]]}}},
|
||||
"options": {"type": {"key": "string", "value": "string",
|
||||
"min": 0, "max": "unlimited"}},
|
||||
"external_ids": {
|
||||
"type": {"key": "string", "value": "string",
|
||||
"min": 0, "max": "unlimited"}}},
|
||||
|
||||
@ -482,6 +482,7 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
|
||||
'loadbalancer_id': self.ref_pool.loadbalancer_id,
|
||||
'listener_id': self.ref_pool.listener_id,
|
||||
'protocol': self.ref_pool.protocol,
|
||||
'lb_algorithm': constants.LB_ALGORITHM_SOURCE_IP_PORT,
|
||||
'admin_state_up': self.ref_pool.admin_state_up}
|
||||
expected_dict = {'type': ovn_const.REQ_TYPE_POOL_CREATE,
|
||||
'info': info}
|
||||
@ -493,6 +494,7 @@ class TestOvnProviderDriver(ovn_base.TestOvnOctaviaBase):
|
||||
info = {'id': self.ref_pool.pool_id,
|
||||
'loadbalancer_id': self.ref_pool.loadbalancer_id,
|
||||
'protocol': self.ref_pool.protocol,
|
||||
'lb_algorithm': constants.LB_ALGORITHM_SOURCE_IP_PORT,
|
||||
'listener_id': self.ref_pool.listener_id,
|
||||
'admin_state_up': True}
|
||||
expected_dict = {'type': ovn_const.REQ_TYPE_POOL_CREATE,
|
||||
|
||||
@ -53,6 +53,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
|
||||
'loadbalancer_id': self.loadbalancer_id,
|
||||
'listener_id': self.listener_id,
|
||||
'protocol': 'TCP',
|
||||
'lb_algorithm': constants.LB_ALGORITHM_SOURCE_IP_PORT,
|
||||
'admin_state_up': False}
|
||||
self.member = {'id': self.member_id,
|
||||
'address': self.member_address,
|
||||
@ -244,6 +245,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
|
||||
expected_lb_info = {
|
||||
'id': self.ovn_lb.name,
|
||||
'protocol': 'tcp',
|
||||
'lb_algorithm': constants.LB_ALGORITHM_SOURCE_IP_PORT,
|
||||
'vip_address': udp_lb.external_ids.get(
|
||||
ovn_const.LB_EXT_IDS_VIP_KEY),
|
||||
'vip_port_id':
|
||||
@ -299,13 +301,34 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
|
||||
ovn_const.LB_EXT_IDS_VIP_PORT_ID_KEY: mock.ANY,
|
||||
'enabled': 'False'},
|
||||
name=mock.ANY,
|
||||
protocol=None)
|
||||
protocol=None,
|
||||
selection_fields=['ip_src', 'ip_dst', 'tp_src', 'tp_dst'])
|
||||
|
||||
@mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
|
||||
def test_lb_create_enabled(self, net_cli):
|
||||
self.lb['admin_state_up'] = True
|
||||
net_cli.return_value.list_ports.return_value = self.ports
|
||||
status = self.helper.lb_create(self.lb)
|
||||
self.assertEqual(status['loadbalancers'][0]['provisioning_status'],
|
||||
constants.ACTIVE)
|
||||
self.assertEqual(status['loadbalancers'][0]['operating_status'],
|
||||
constants.ONLINE)
|
||||
self.helper.ovn_nbdb_api.db_create.assert_called_once_with(
|
||||
'Load_Balancer', external_ids={
|
||||
ovn_const.LB_EXT_IDS_VIP_KEY: mock.ANY,
|
||||
ovn_const.LB_EXT_IDS_VIP_PORT_ID_KEY: mock.ANY,
|
||||
'enabled': 'True'},
|
||||
name=mock.ANY,
|
||||
protocol=None,
|
||||
selection_fields=['ip_src', 'ip_dst', 'tp_src', 'tp_dst'])
|
||||
|
||||
@mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
|
||||
def test_lb_create_selection_fields_not_supported(self, net_cli):
|
||||
self.lb['admin_state_up'] = True
|
||||
net_cli.return_value.list_ports.return_value = self.ports
|
||||
self.helper._are_selection_fields_supported = (
|
||||
mock.Mock(return_value=False))
|
||||
status = self.helper.lb_create(self.lb)
|
||||
self.assertEqual(status['loadbalancers'][0]['provisioning_status'],
|
||||
constants.ACTIVE)
|
||||
self.assertEqual(status['loadbalancers'][0]['operating_status'],
|
||||
@ -318,6 +341,27 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
|
||||
name=mock.ANY,
|
||||
protocol=None)
|
||||
|
||||
@mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
|
||||
def test_lb_create_selection_fields_not_supported_algo(self, net_cli):
|
||||
self.lb['admin_state_up'] = True
|
||||
net_cli.return_value.list_ports.return_value = self.ports
|
||||
self.pool['lb_algoritm'] = 'foo'
|
||||
status = self.helper.lb_create(self.lb)
|
||||
self.assertEqual(status['loadbalancers'][0]['provisioning_status'],
|
||||
constants.ACTIVE)
|
||||
self.assertEqual(status['loadbalancers'][0]['operating_status'],
|
||||
constants.ONLINE)
|
||||
# NOTE(mjozefcz): Make sure that we use the same selection
|
||||
# fields as for default algorithm - source_ip_port.
|
||||
self.helper.ovn_nbdb_api.db_create.assert_called_once_with(
|
||||
'Load_Balancer', external_ids={
|
||||
ovn_const.LB_EXT_IDS_VIP_KEY: mock.ANY,
|
||||
ovn_const.LB_EXT_IDS_VIP_PORT_ID_KEY: mock.ANY,
|
||||
'enabled': 'True'},
|
||||
name=mock.ANY,
|
||||
protocol=None,
|
||||
selection_fields=['ip_src', 'ip_dst', 'tp_src', 'tp_dst'])
|
||||
|
||||
@mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
|
||||
def test_lb_create_on_multi_protocol(self, net_cli):
|
||||
"""This test situation when new protocol is added
|
||||
@ -342,7 +386,8 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
|
||||
ovn_const.LB_EXT_IDS_LR_REF_KEY: 'foo',
|
||||
'enabled': 'True'},
|
||||
name=mock.ANY,
|
||||
protocol='udp')
|
||||
protocol='udp',
|
||||
selection_fields=['ip_src', 'ip_dst', 'tp_src', 'tp_dst'])
|
||||
self.helper._update_lb_to_ls_association.assert_has_calls([
|
||||
mock.call(self.ovn_lb, associate=True,
|
||||
network_id=self.lb['vip_network_id']),
|
||||
|
||||
Loading…
Reference in New Issue
Block a user