openstack/neutron
Code Issues Proposed changes

Improve the `PortBindingUpdateVirtualPortsEvent` match filter

Browse Source 
This patch improves the ``PortBindingUpdateVirtualPortsEvent``
match filter. These are the new conditions:
* Event delete: that happens when the port binding has been deleted
  because the port is no longer bound or the port has been
  deleted. That will remove the Neutron port host name.
  NOTE: in case the Neutron port has been deleted, the method
  ``Ml2Plugin.update_virtual_port_host`` won't update (create) a new
  PortBinding object.
* If the new register has virtual_parents but not the old one, that
  means the ovn-controller has received traffic with the VIP from
  this port. The port host ID must be set.
* If the virtual parents have changed, the port host ID must be
  updated.
* If the virtual parents have been removed, the port host ID must
  be removed too.

Newer versions of OVN [1] are handling the virtual port binding in
a different way. When the virtual parents are added or removed,
the related "Port_Binding" register is deleted and the created
again. This is why this new version includes the event "DELETE"
on the match method; when the register is deleted, the event
class considers that the port is no longer bound to a host and
removes the host name for the Neutron port.

[1]https://review.opendev.org/c/openstack/neutron/+/880890/

Change-Id: I34caf7d0212ccb4bd7259c4414e7c3994bd8da4d
(cherry picked from commit 2fbfe3855e)
This commit is contained in:
Rodolfo Alonso Hernandez 2023-05-19 04:21:02 +02:00 committed by Mohammed Naser
parent 0cbfe6cbd1
commit 810c813adf
3 changed files with 74 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py
View File

@ -547,43 +547,42 @@ class PortBindingUpdateVirtualPortsEvent(row_event.RowEvent):
def __init__(self, driver):
self.driver = driver
table = 'Port_Binding'
events = (self.ROW_UPDATE, )
events = (self.ROW_UPDATE, self.ROW_DELETE)
super().__init__(events, table, None)
self.event_name = 'PortBindingUpdateVirtualPortsEvent'
def match_fn(self, event, row, old):
# This event should catch only those events from ports that are
# "virtual" or have been "virtual". The second happens when all virtual
# parent are disassociated; in the same transaction the
# "virtual-parents" list is removed from "options" and the type is set
# to "".
if (row.type != ovn_const.PB_TYPE_VIRTUAL and
getattr(old, 'type', None) != ovn_const.PB_TYPE_VIRTUAL):
return False
# This event should catch the events related to virtual parents (that
# are associated to virtual ports).
if event == self.ROW_DELETE:
# The port binding has been deleted, delete the host ID (if the
# port was not deleted before).
return True
virtual_parents = (row.options or {}).get(
ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY)
old_virtual_parents = getattr(old, 'options', {}).get(
ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY)
chassis = row.chassis
old_chassis = getattr(old, 'chassis', [])
if virtual_parents and chassis != old_chassis:
# That happens when the chassis is assigned (VIP is first detected
# in a port) or changed (the VIP changes of assigned port and
# host).
return True
if not virtual_parents and old_virtual_parents:
if virtual_parents != old_virtual_parents:
# 1) if virtual_parents and not old_virtual_parents:
# The port has received a virtual parent and now is bound.
# 2) elif (virtual_parents and old_virtual_parents and
# old_virtual_parents != virtual_parents):
# If the port virtual parents have changed (the VIP is bound
# to another host because it's owned by another port).
# 3) if not virtual_parents and old_virtual_parents:
# All virtual parent ports are removed, the VIP is unbound.
return True
return False
def run(self, event, row, old):
virtual_parents = (row.options or {}).get(
ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY)
chassis_uuid = (row.chassis[0].uuid if
row.chassis and virtual_parents else None)
if event == self.ROW_DELETE:
chassis_uuid = None
else:
virtual_parents = (row.options or {}).get(
ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY)
chassis_uuid = (row.chassis[0].uuid if
row.chassis and virtual_parents else None)
self.driver.update_virtual_port_host(row.logical_port, chassis_uuid)

10
neutron/plugins/ml2/plugin.py
View File

@ -2067,8 +2067,14 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
"""
hostname = hostname or ''
with db_api.CONTEXT_WRITER.using(context):
for pb in ports_obj.PortBinding.get_objects(context,
port_id=port_id):
pbindings = ports_obj.PortBinding.get_objects(context,
port_id=port_id)
if not pbindings:
# The port has been deleted and there is no need to delete and
# create any port binding.
return
for pb in pbindings:
pb.delete()
attrs = {'port_id': port_id,

45
neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py
View File

@ -311,6 +311,17 @@ class TestNBDbMonitor(base.TestOVNFunctionalBase):
rows = cmd.execute(check_error=True)
return rows[0] if rows else None
def _set_port_binding_virtual_parent(self, port_id, parent_port_id):
pb_port_parent = self.sb_api.db_find_rows(
'Port_Binding', ('logical_port', '=', parent_port_id)).execute(
check_error=True)[0]
pb_port_vip = self.sb_api.db_find_rows(
'Port_Binding', ('logical_port', '=', port_id)).execute(
check_error=True)[0]
self.sb_api.db_set(
'Port_Binding', pb_port_vip.uuid,
('virtual_parent', pb_port_parent.uuid)).execute(check_error=True)
def _check_port_binding_type(self, port_id, port_type):
def is_port_binding_type(port_id, port_type):
bp = self._find_port_binding(port_id)
@ -340,6 +351,7 @@ class TestNBDbMonitor(base.TestOVNFunctionalBase):
port = self.create_port()
self._check_port_binding_type(vip['id'], '')
# 1) Set the allowed address pairs.
data = {'port': {'allowed_address_pairs': allowed_address_pairs}}
req = self.new_update_request('ports', data, port['id'])
req.get_response(self.api)
@ -347,8 +359,21 @@ class TestNBDbMonitor(base.TestOVNFunctionalBase):
# and the corresponding "virtual-parents".
self._check_port_binding_type(vip['id'], ovn_const.LSP_TYPE_VIRTUAL)
self._check_port_virtual_parents(vip['id'], port['id'])
mock_update_vip_host.assert_not_called()
n_utils.wait_until_true(lambda: mock_update_vip_host.called,
timeout=10)
# The "Port_Binding" has been deleted. Then the "Port_Binding" register
# is created again without virtual_parents, but this event doesn't
# call "update_virtual_port_host".
mock_update_vip_host.assert_called_once_with(vip['id'], None)
# 2) Unset the allowed address pairs.
# Assign the VIP again and delete the virtual port.
# Before unsetting the allowed address pairs, we first manually add
# the Port_Binding.virtual_parent of the virtual port. That happens
# when an ovn-controller detects traffic with the VIP and assign the
# port hosting the VIP as virtual parent.
self._set_port_binding_virtual_parent(vip['id'], port['id'])
mock_update_vip_host.reset_mock()
data = {'port': {'allowed_address_pairs': []}}
req = self.new_update_request('ports', data, port['id'])
req.get_response(self.api)
@ -356,6 +381,24 @@ class TestNBDbMonitor(base.TestOVNFunctionalBase):
self._check_port_virtual_parents(vip['id'], None)
n_utils.wait_until_true(lambda: mock_update_vip_host.called,
timeout=10)
# The virtual port is no longer considered as virtual. The
# "Port_Binding" register is deleted.
mock_update_vip_host.assert_called_once_with(vip['id'], None)
# 3) Set again the allowed address pairs.
mock_update_vip_host.reset_mock()
data = {'port': {'allowed_address_pairs': allowed_address_pairs}}
req = self.new_update_request('ports', data, port['id'])
req.get_response(self.api)
# This test checks that the VIP "Port_Binding" register gets the type
# and the corresponding "virtual-parents".
self._check_port_binding_type(vip['id'], ovn_const.LSP_TYPE_VIRTUAL)
self._check_port_virtual_parents(vip['id'], port['id'])
mock_update_vip_host.reset_mock()
self._delete('ports', vip['id'])
n_utils.wait_until_true(lambda: mock_update_vip_host.called,
timeout=10)
# The virtual port is deleted and so the associated "Port_Binding".
mock_update_vip_host.assert_called_once_with(vip['id'], None)