From 6b6abd0718746ca55c2eb302570b874b7e2362c8 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 9 Feb 2022 19:04:45 +0000 Subject: [PATCH] [OVN] Update VIP port host ID when traffic detected A virtual IP port is a port with an IP address assigned. This IP address is used as an allowed address pair in a bound port (or ports). This port is marked as "virtual" and a list of "virtual-parents" (ports with the VIP assigned as allowed address pair) will be populated accordingly. This patch updates the "binding:host_id" of the port when OVN detects traffic from the corresponding VIP address. OVN updates the VIP port SB "Port_Binding" register with the chassis ID. The hostname of this chassis is used to update the port host ID. The VIP port is always unbound and down; this patch only populates the host information to provide to the user this information using the OpenStack CLI, instead of making this search using the OVN CLI. Closes-Bug: #1961184 Change-Id: I75b04d056ba0df9e34a99673c689a69cdbfa097e (cherry picked from commit bdbabdf362a0755fa5a86222a7a223c727f0ec4e) --- .../drivers/ovn/mech_driver/mech_driver.py | 9 ++ .../ovn/mech_driver/ovsdb/ovsdb_monitor.py | 53 +++++++++++- neutron/plugins/ml2/plugin.py | 24 ++++++ .../mech_driver/ovsdb/test_ovsdb_monitor.py | 84 ++++++++++++++++--- 4 files changed, 156 insertions(+), 14 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index fc145466774..d6d60ea49f2 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -1001,6 +1001,15 @@ class OVNMechanismDriver(api.MechanismDriver): vif_details) break + def update_virtual_port_host(self, port_id, chassis_id): + if chassis_id: + hostname = self.sb_ovn.db_get( + 'Chassis', chassis_id, 'hostname').execute(check_error=True) + else: + hostname = '' + self._plugin.update_virtual_port_host(n_context.get_admin_context(), + port_id, hostname) + def get_workers(self): """Get any worker instances that should have their own process diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py index 9e103bd348a..02ba4e3810c 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py @@ -582,6 +582,55 @@ class PortBindingUpdateDownEvent(row_event.RowEvent): self.driver.set_port_status_down(row.logical_port) +class PortBindingUpdateVirtualPortsEvent(row_event.RowEvent): + """Row update event - Port_Binding for virtual ports + + The goal of this event is to catch the events of the virtual ports and + update the hostname in the related "portbinding" register. + """ + def __init__(self, driver): + self.driver = driver + table = 'Port_Binding' + events = (self.ROW_UPDATE, ) + 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 + + 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: + # 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) + self.driver.update_virtual_port_host(row.logical_port, chassis_uuid) + + class FIPAddDeleteEvent(row_event.RowEvent): """Row event - NAT 'dnat_and_snat' entry added or deleted @@ -797,7 +846,9 @@ class OvnSbIdl(OvnIdlDistributedLock): self._pb_create_up_event, self._pb_create_down_event, PortBindingUpdateUpEvent(driver), - PortBindingUpdateDownEvent(driver)]) + PortBindingUpdateDownEvent(driver), + PortBindingUpdateVirtualPortsEvent(driver), + ]) @classmethod def from_server(cls, connection_string, helper, driver): diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index e0231f441af..553bdc6e246 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -1955,6 +1955,30 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, return self._bind_port_if_needed(mech_context) + @utils.transaction_guard + @db_api.retry_if_session_inactive() + def update_virtual_port_host(self, context, port_id, hostname): + """Create a new portbinding register with the updated hostname + + A virtual port is not actually bound, therefore the portbinding default + register is always VIF_TYPE_UNBOUND. However this method updates the + host information to reflect where the associated port that is sending + or receiving traffic, using the VIP address, is hosted. + """ + hostname = hostname or '' + with db_api.CONTEXT_WRITER.using(context): + for pb in ports_obj.PortBinding.get_objects(context, + port_id=port_id): + pb.delete() + + attrs = {'port_id': port_id, + 'vnic_type': portbindings.VNIC_NORMAL, + 'vif_details': {}, + 'profile': {}, + 'vif_type': portbindings.VIF_TYPE_UNBOUND, + 'host': hostname} + ports_obj.PortBinding(context, **attrs).create() + def _pre_delete_port(self, context, port_id, port_check, port=None): """Do some preliminary operations before deleting the port.""" LOG.debug("Deleting port %s", port_id) diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py index e93d743cc70..0f272d95a4a 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py @@ -12,26 +12,30 @@ # License for the specific language governing permissions and limitations # under the License. +import functools from unittest import mock import fixtures as og_fixtures +from neutron_lib.api.definitions import allowedaddresspairs +from neutron_lib.api.definitions import portbindings +from neutron_lib.plugins import constants as plugin_constants +from neutron_lib.plugins import directory from oslo_concurrency import processutils from oslo_utils import uuidutils +from ovsdbapp.backend.ovs_idl import event +from ovsdbapp.backend.ovs_idl import idlutils + from neutron.common.ovn import constants as ovn_const from neutron.common import utils as n_utils from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.db import ovn_hash_ring_db as db_hash_ring from neutron.plugins.ml2.drivers.ovn.agent import neutron_agent +from neutron.plugins.ml2.drivers.ovn.mech_driver import mech_driver from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovsdb_monitor from neutron.tests.functional import base from neutron.tests.functional.resources.ovsdb import fixtures from neutron.tests.functional.resources import process -from neutron_lib.api.definitions import portbindings -from neutron_lib.plugins import constants as plugin_constants -from neutron_lib.plugins import directory -from ovsdbapp.backend.ovs_idl import event -from ovsdbapp.backend.ovs_idl import idlutils class WaitForDataPathBindingCreateEvent(event.WaitEvent): @@ -80,16 +84,21 @@ class TestNBDbMonitor(base.TestOVNFunctionalBase): super(TestNBDbMonitor, self).setUp() self.chassis = self.add_fake_chassis('ovs-host1') self.l3_plugin = directory.get_plugin(plugin_constants.L3) + self.net = self._make_network(self.fmt, 'net1', True) + self._make_subnet(self.fmt, self.net, '20.0.0.1', '20.0.0.0/24', + ip_version=4) - def create_port(self): - net = self._make_network(self.fmt, 'net1', True) - self._make_subnet(self.fmt, net, '20.0.0.1', - '20.0.0.0/24', ip_version=4) - arg_list = ('device_owner', 'device_id', portbindings.HOST_ID) - host_arg = {'device_owner': 'compute:nova', + def create_port(self, device_owner='compute:nova', host='ovs-host1', + allowed_address_pairs=None): + allowed_address_pairs = allowed_address_pairs or [] + arg_list = ('device_owner', 'device_id', portbindings.HOST_ID, + allowedaddresspairs.ADDRESS_PAIRS) + host_arg = {'device_owner': device_owner, 'device_id': uuidutils.generate_uuid(), - portbindings.HOST_ID: 'ovs-host1'} - port_res = self._create_port(self.fmt, net['network']['id'], + portbindings.HOST_ID: host, + allowedaddresspairs.ADDRESS_PAIRS: allowed_address_pairs + } + port_res = self._create_port(self.fmt, self.net['network']['id'], arg_list=arg_list, **host_arg) port = self.deserialize(self.fmt, port_res)['port'] return port @@ -302,6 +311,55 @@ class TestNBDbMonitor(base.TestOVNFunctionalBase): "Fanout event didn't get handled expected %d times" % (worker_num + 1))) + def _find_port_binding(self, port_id): + cmd = self.sb_api.db_find_rows('Port_Binding', + ('logical_port', '=', port_id)) + rows = cmd.execute(check_error=True) + return rows[0] if rows else None + + 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) + return port_type == bp.type + + check = functools.partial(is_port_binding_type, port_id, port_type) + n_utils.wait_until_true(check, timeout=10) + + def _check_port_virtual_parents(self, port_id, vparents): + def is_port_virtual_parents(port_id, vparents): + bp = self._find_port_binding(port_id) + return (vparents == + bp.options.get(ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY)) + + check = functools.partial(is_port_virtual_parents, port_id, vparents) + n_utils.wait_until_true(check, timeout=10) + + @mock.patch.object(mech_driver.OVNMechanismDriver, + 'update_virtual_port_host') + def test_virtual_port_host_update(self, mock_update_vip_host): + # NOTE: because we can't simulate traffic from a port, this check is + # not done in this test. This test checks the VIP host is unset when + # the port allowed address pairs are removed. + vip = self.create_port(device_owner='', host='') + vip_address = vip['fixed_ips'][0]['ip_address'] + allowed_address_pairs = [{'ip_address': vip_address}] + port = self.create_port() + self._check_port_binding_type(vip['id'], '') + + 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) + mock_update_vip_host.assert_not_called() + + data = {'port': {'allowed_address_pairs': []}} + req = self.new_update_request('ports', data, port['id']) + req.get_response(self.api) + self._check_port_binding_type(vip['id'], '') + mock_update_vip_host.assert_called_once_with(vip['id'], None) + class TestNBDbMonitorOverTcp(TestNBDbMonitor): def get_ovsdb_server_protocol(self):