openstack
/
ovn-octavia-provider
Code Issues Proposed changes

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

 Conflicts:
        ovn_octavia_provider/common/constants.py
        ovn_octavia_provider/helper.py
        ovn_octavia_provider/tests/functional/base.py
        ovn_octavia_provider/tests/unit/test_helper.py

Change-Id: I7b4ab99d1be2855e18b186557990c85f170ad548
Closes-Bug: #1871239
(cherry picked from commit 23d743a444)
This commit is contained in:
Maciej Józefczyk 2020-05-11 10:16:56 +00:00
parent 740164259b
commit fdc7776445
5 changed files with 170 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
ovn_octavia_provider/common/constants.py
View File

@ -10,6 +10,9 @@
# License for the specific language governing permissions and limitations
# under the License.
from octavia_lib.common import constants
# TODO(mjozefcz): Use those variables from neutron-lib once released.
LRP_PREFIX = "lrp-"
LB_VIP_PORT_PREFIX = "ovn-lb-vip-"
@ -36,3 +39,11 @@ LB_EXT_IDS_VIP_PORT_ID_KEY = 'neutron:vip_port_id'
# Auth sections
SERVICE_AUTH = 'service_auth'
# 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"],
}

34
ovn_octavia_provider/driver.py
View File

@ -514,12 +514,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()
@ -546,6 +551,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':
@ -891,6 +897,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:
@ -934,12 +948,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['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['id'],
protocol=protocol)
@ -1350,7 +1370,8 @@ class OvnProviderHelper(object):
ovn_lb = self._get_or_create_ovn_lb(
pool['loadbalancer_id'],
pool['protocol'],
pool['admin_state_up'])
pool['admin_state_up'],
lb_algorithm=pool[constants.LB_ALGORITHM])
external_ids = copy.deepcopy(ovn_lb.external_ids)
pool_key = self._get_pool_key(pool['id'],
is_enabled=pool['admin_state_up'])
@ -1982,6 +2003,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': REQ_TYPE_POOL_CREATE,

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

@ -178,9 +178,13 @@ class TestOvnOctaviaBase(
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):
@ -436,6 +440,12 @@ class TestOvnOctaviaBase(
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 []
@ -458,17 +468,16 @@ class TestOvnOctaviaBase(
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.

64
ovn_octavia_provider/tests/unit/schemas/ovn-nb.ovsschema
View File

@ -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"}}},

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

@ -585,6 +585,7 @@ class TestOvnProviderDriver(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_driver.REQ_TYPE_POOL_CREATE,
'info': info}
@ -596,6 +597,7 @@ class TestOvnProviderDriver(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_driver.REQ_TYPE_POOL_CREATE,
@ -698,6 +700,7 @@ class TestOvnProviderHelper(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,
@ -889,6 +892,7 @@ class TestOvnProviderHelper(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':
@ -944,13 +948,34 @@ class TestOvnProviderHelper(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.driver.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.driver.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'],
@ -963,6 +988,27 @@ class TestOvnProviderHelper(TestOvnOctaviaBase):
name=mock.ANY,
protocol=None)
@mock.patch('ovn_octavia_provider.driver.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.driver.get_neutron_client')
def test_lb_create_on_multi_protocol(self, net_cli):
"""This test situation when new protocol is added
@ -987,7 +1033,8 @@ class TestOvnProviderHelper(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']),