diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py index 1d2f3146f44..999ad3cd655 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -309,6 +309,25 @@ def _load_config(self): LOG.debug("Loaded chassis name %s (UUID: %s) and ovn bridge %s.", self.chassis, self.chassis_id, self.ovn_bridge) + def _update_chassis_config(self): + """Update the Chassis register information + + This method should be called once the Metadata Agent has been + registered (method ``register_metadata_agent`` has been called) and + the corresponding Chasis or Chassis_Private register has been + created/updated. The Chassis table is used in an event when OVN does + not have the Chassis_Private. + """ + external_ids = {ovn_const.OVN_AGENT_OVN_BRIDGE: self.ovn_bridge} + if self.has_chassis_private: + self.sb_idl.db_set( + 'Chassis_Private', self.chassis, + ('external_ids', external_ids)).execute(check_error=True) + else: + self.sb_idl.db_set( + 'Chassis', self.chassis, + ('external_ids', external_ids)).execute(check_error=True) + @_sync_lock def resync(self): """Resync the agent. @@ -316,6 +335,7 @@ def resync(self): Reload the configuration and sync the agent again. """ self._load_config() + self._update_chassis_config() self.sync() def start(self): @@ -361,6 +381,7 @@ def start(self): # Register the agent with its corresponding Chassis self.register_metadata_agent() + self._update_chassis_config() self._proxy.wait() diff --git a/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py b/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py index dc3963f4ef3..488bb228c98 100644 --- a/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py +++ b/neutron/api/rpc/agentnotifiers/dhcp_rpc_agent_api.py @@ -284,6 +284,7 @@ def _after_router_interface_deleted(self, resource, event, trigger, port = payload.metadata.get('port') self._notify_agents(payload.context, 'port_delete_end', {'port_id': port['id'], + 'fixed_ips': port['fixed_ips'], 'network_id': port['network_id']}, port['network_id']) diff --git a/neutron/common/_constants.py b/neutron/common/_constants.py index 03c07eb71a4..59d5c96df1c 100644 --- a/neutron/common/_constants.py +++ b/neutron/common/_constants.py @@ -89,3 +89,6 @@ # Neutron-lib defines this with a /64 but it should be /128 METADATA_V6_CIDR = constants.METADATA_V6_IP + '/128' + +# TODO(ralonsoh): move this constant to neutron_lib.plugins.ml2.ovs_constants +DEFAULT_BR_INT = 'br-int' diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index 3d4299ebe3b..3827c55643c 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -71,6 +71,8 @@ OVN_NAME_PREFIX = 'neutron-' OVN_HA_CH_GROUP_EXTPORT_PREFIX = 'neutron-extport-' +OVN_DATAPATH_TYPE = 'datapath-type' + # TODO(froyo): Move this to neutron-lib as soon as possible, and when a new # release is created and pointed to in the requirements remove this code OVN_LB_HM_PORT_DISTRIBUTED = 'ovn-lb-hm:distributed' @@ -83,6 +85,7 @@ OVN_AGENT_NEUTRON_SB_CFG_KEY = 'neutron:ovn-neutron-agent-sb-cfg' OVN_AGENT_NEUTRON_DESC_KEY = 'neutron:description-neutron-agent' OVN_AGENT_NEUTRON_ID_KEY = 'neutron:ovn-neutron-agent-id' +OVN_AGENT_OVN_BRIDGE = 'neutron:ovn-bridge' OVN_CONTROLLER_AGENT = 'OVN Controller agent' OVN_CONTROLLER_GW_AGENT = 'OVN Controller Gateway agent' OVN_METADATA_AGENT = 'OVN Metadata agent' diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 19d887a6f17..d7a0f0cc74c 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -40,6 +40,7 @@ import tenacity from neutron._i18n import _ +from neutron.common import _constants as n_const from neutron.common.ovn import constants from neutron.common.ovn import exceptions as ovn_exc from neutron.common import utils as common_utils @@ -367,7 +368,7 @@ def validate_and_get_data_from_binding_profile(port): if (constants.OVN_PORT_BINDING_PROFILE not in port or not validators.is_attr_set( port[constants.OVN_PORT_BINDING_PROFILE])): - BPInfo({}, None, []) + return BPInfo({}, None, []) param_set = {} param_dict = {} vnic_type = port.get(portbindings.VNIC_TYPE, portbindings.VNIC_NORMAL) @@ -718,6 +719,32 @@ def get_ovn_cms_options(chassis): constants.OVN_CMS_OPTIONS, '').split(',')] +def get_ovn_bridge_from_chassis(chassis): + """Return the OVN bridge used by the local OVN controller + + This information is stored in the Chassis or Chassis_Private register by + the OVN Metadata agent. The default value returned, if not present, is + "br-int". + NOTE: the default value is not reading the local ``OVS.integration_bridge`` + configuration knob, that could be different. + """ + return (chassis.external_ids.get(constants.OVN_AGENT_OVN_BRIDGE) or + n_const.DEFAULT_BR_INT) + + +def get_datapath_type(hostname, sb_idl): + """Return the local OVS integration bridge datapath type + + If the datapath type is not stored in the ``Chassis`` register or + the register is still not created, the default value returned is "". + """ + chassis = sb_idl.db_find( + 'Chassis', ('hostname', '=', hostname)).execute(check_error=True) + return ( + chassis[0].get('other_config', {}).get(constants.OVN_DATAPATH_TYPE, '') + if chassis else '') + + def is_gateway_chassis(chassis): """Check if the given chassis is a gateway chassis""" return constants.CMS_OPT_CHASSIS_AS_GW in get_ovn_cms_options(chassis) diff --git a/neutron/db/models_v2.py b/neutron/db/models_v2.py index 21eb46ff72d..10c42f920ee 100644 --- a/neutron/db/models_v2.py +++ b/neutron/db/models_v2.py @@ -308,7 +308,7 @@ class SubnetPool(standard_attr.HasStandardAttributes, model_base.BASEV2, lazy='subquery') rbac_entries = sa.orm.relationship(rbac_db_models.SubnetPoolRBAC, backref='subnetpools', - lazy='joined', + lazy='selectin', cascade='all, delete, delete-orphan') api_collections = [subnetpool_def.COLLECTION_NAME] collection_resource_map = {subnetpool_def.COLLECTION_NAME: diff --git a/neutron/plugins/ml2/drivers/mech_sriov/agent/sriov_nic_agent.py b/neutron/plugins/ml2/drivers/mech_sriov/agent/sriov_nic_agent.py index e552aba48a1..d9e82ed1a88 100644 --- a/neutron/plugins/ml2/drivers/mech_sriov/agent/sriov_nic_agent.py +++ b/neutron/plugins/ml2/drivers/mech_sriov/agent/sriov_nic_agent.py @@ -112,17 +112,39 @@ def network_update(self, context, **kwargs): def binding_activate(self, context, **kwargs): if kwargs.get('host') != self.agent.conf.host: return - LOG.debug("binding activate for port %s", kwargs.get('port_id')) - device_details = self.agent.get_device_details_from_port_id( - kwargs.get('port_id')) - mac = device_details.get('mac_address') - binding_profile = device_details.get('profile') - if binding_profile: - pci_slot = binding_profile.get('pci_slot') - self.agent.activated_bindings.add((mac, pci_slot)) + + port_id = kwargs.get('port_id') + + def _is_port_id_in_network(network_port, port_id): + for network_id, ports in network_port.items(): + for port in ports: + if port['port_id'] == port_id: + return True + return False + + is_port_id_sriov = _is_port_id_in_network( + self.agent.network_ports, port_id + ) + + if is_port_id_sriov: + LOG.debug("binding activate for port %s", port_id) + device_details = self.agent.get_device_details_from_port_id( + port_id) + mac = device_details.get('mac_address') + binding_profile = device_details.get('profile') + if binding_profile: + pci_slot = binding_profile.get('pci_slot') + self.agent.activated_bindings.add((mac, pci_slot)) + else: + LOG.warning( + "binding_profile not found for port %s.", + port_id + ) else: - LOG.warning("binding_profile not found for port %s.", - kwargs.get('port_id')) + LOG.warning( + "This port is not SRIOV, skip binding for port %s.", + port_id + ) def binding_deactivate(self, context, **kwargs): if kwargs.get('host') != self.agent.conf.host: diff --git a/neutron/plugins/ml2/drivers/ovn/db_migration.py b/neutron/plugins/ml2/drivers/ovn/db_migration.py index a4f386a1837..47df92097c9 100644 --- a/neutron/plugins/ml2/drivers/ovn/db_migration.py +++ b/neutron/plugins/ml2/drivers/ovn/db_migration.py @@ -11,6 +11,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import copy from neutron_lib.api.definitions import portbindings as pb_api from neutron_lib import context as n_context @@ -20,6 +21,7 @@ from oslo_log import log as logging from sqlalchemy.orm import exc as sqla_exc +from neutron.common import _constants as n_const from neutron.db.models.plugins.ml2 import geneveallocation from neutron.db.models.plugins.ml2 import vxlanallocation from neutron.objects import network as network_obj @@ -29,10 +31,6 @@ LOG = logging.getLogger(__name__) -VIF_DETAILS_TO_REMOVE = ( - pb_api.VIF_DETAILS_BRIDGE_NAME, -) - def migrate_neutron_database_to_ovn(): """Change DB content from OVS to OVN mech driver. @@ -74,18 +72,22 @@ def migrate_neutron_database_to_ovn(): with db_api.CONTEXT_WRITER.using(ctx): pb = port_obj.PortBinding.get_object(ctx, port_id=port_id, host=host) - if not pb or not pb.vif_details: + if not pb: continue - vif_details = pb.vif_details.copy() - for detail in VIF_DETAILS_TO_REMOVE: - try: - del vif_details[detail] - except KeyError: - pass - if vif_details == pb.vif_details: + # Update the OVS bridge name in the VIF details: now all + # port are directly connected to the integration bridge. + # Because the name of each host integration bridge is not + # know by the Neutron API at this point, the default value + # "br-int" will be used. + # The OVS datapath type is unchanged. + vif_details = copy.deepcopy(pb.vif_details) or {} + if (vif_details.get(pb_api.VIF_DETAILS_BRIDGE_NAME) == + n_const.DEFAULT_BR_INT): continue + vif_details[pb_api.VIF_DETAILS_BRIDGE_NAME] = ( + n_const.DEFAULT_BR_INT) pb.vif_details = vif_details pb.update() except (exceptions.ObjectNotFound, 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 4c0f8bdc7bc..1426ff39091 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -190,7 +190,7 @@ def get_supported_vif_types(self): vif_types = set() for ch in self.sb_ovn.chassis_list().execute(check_error=True): other_config = ovn_utils.get_ovn_chassis_other_config(ch) - dp_type = other_config.get('datapath-type', '') + dp_type = other_config.get(ovn_const.OVN_DATAPATH_TYPE, '') if dp_type == ovn_const.CHASSIS_DATAPATH_NETDEV: vif_types.add(portbindings.VIF_TYPE_VHOST_USER) else: @@ -989,7 +989,7 @@ def bind_port(self, context): return chassis = agent.chassis other_config = ovn_utils.get_ovn_chassis_other_config(chassis) - datapath_type = other_config.get('datapath-type', '') + datapath_type = other_config.get(ovn_const.OVN_DATAPATH_TYPE, '') iface_types = other_config.get('iface-types', '') iface_types = iface_types.split(',') if iface_types else [] chassis_physnets = self.sb_ovn._get_chassis_physnets(chassis) @@ -1036,13 +1036,25 @@ def bind_port(self, context): vif_type = portbindings.VIF_TYPE_VHOST_USER port[portbindings.VIF_DETAILS].update({ portbindings.VHOST_USER_SOCKET: vhost_user_socket}) - vif_details = dict(self.vif_details[vif_type]) + vif_details = copy.deepcopy(self.vif_details[vif_type]) vif_details[portbindings.VHOST_USER_SOCKET] = ( vhost_user_socket) else: vif_type = portbindings.VIF_TYPE_OVS - vif_details = self.vif_details[vif_type] + vif_details = copy.deepcopy(self.vif_details[vif_type]) + if self.agent_chassis_table == 'Chassis_Private': + chassis_to_retrieve = agent.chassis_private + else: + chassis_to_retrieve = agent.chassis + ovn_bridge = ovn_utils.get_ovn_bridge_from_chassis( + chassis_to_retrieve) + + dp_type = ovn_utils.get_datapath_type(bind_host, self.sb_ovn) + vif_details.update({ + portbindings.VIF_DETAILS_BRIDGE_NAME: ovn_bridge, + portbindings.OVS_DATAPATH_TYPE: dp_type, + }) context.set_binding(segment_to_bind[api.ID], vif_type, vif_details) break diff --git a/neutron/tests/functional/agent/ovn/metadata/test_metadata_agent.py b/neutron/tests/functional/agent/ovn/metadata/test_metadata_agent.py index f1142a39592..39fb43dd7b8 100644 --- a/neutron/tests/functional/agent/ovn/metadata/test_metadata_agent.py +++ b/neutron/tests/functional/agent/ovn/metadata/test_metadata_agent.py @@ -110,6 +110,11 @@ def _start_metadata_agent(self): with mock.patch.object(metadata_server.UnixDomainMetadataProxy, 'wait'): agt.start() + external_ids = agt.sb_idl.db_get( + 'Chassis_Private', agt.chassis, 'external_ids').execute( + check_error=True) + self.assertEqual(external_ids[ovn_const.OVN_AGENT_OVN_BRIDGE], + self.OVN_BRIDGE) # Metadata agent will open connections to OVS and SB databases. # Close connections to them when the test ends, diff --git a/neutron/tests/functional/base.py b/neutron/tests/functional/base.py index bd515254c78..a873353ea7a 100644 --- a/neutron/tests/functional/base.py +++ b/neutron/tests/functional/base.py @@ -408,8 +408,12 @@ def stop(self): if self.maintenance_worker: self.mech_driver.nb_synchronizer.stop() self.mech_driver.sb_synchronizer.stop() - self.mech_driver.nb_ovn.ovsdb_connection.stop() - self.mech_driver.sb_ovn.ovsdb_connection.stop() + for ovn_conn in (self.mech_driver.nb_ovn.ovsdb_connection, + self.mech_driver.sb_ovn.ovsdb_connection): + try: + ovn_conn.stop(timeout=10) + except Exception: # pylint:disable=bare-except + pass def restart(self): self.stop() diff --git a/neutron/tests/functional/plugins/ml2/drivers/macvtap/agent/test_macvtap_neutron_agent.py b/neutron/tests/functional/plugins/ml2/drivers/macvtap/agent/test_macvtap_neutron_agent.py index 4f0d0b068d3..e4229bde1cb 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/macvtap/agent/test_macvtap_neutron_agent.py +++ b/neutron/tests/functional/plugins/ml2/drivers/macvtap/agent/test_macvtap_neutron_agent.py @@ -48,7 +48,7 @@ def test_get_all_devices(self): try: common_utils.wait_until_true( lambda: {macvtap.link.address} == self.mgr.get_all_devices(), - timeout=5) + timeout=10) except common_utils.WaitTimeout: msg = 'MacVTap address: %s, read devices: %s\n' % ( macvtap.link.address, self.mgr.get_all_devices()) diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 66c5235e3e8..bbe788ebdcb 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -43,6 +43,8 @@ portbindings.CAP_PORT_FILTER: True, portbindings.VIF_DETAILS_CONNECTIVITY: portbindings.CONNECTIVITY_L2, portbindings.VIF_DETAILS_BOUND_DRIVERS: {'0': 'ovn'}, + portbindings.VIF_DETAILS_BRIDGE_NAME: 'br-int', + portbindings.OVS_DATAPATH_TYPE: 'system', } VHOSTUSER_VIF_DETAILS = { portbindings.CAP_PORT_FILTER: False, @@ -50,34 +52,39 @@ 'vhostuser_ovs_plug': True, portbindings.VIF_DETAILS_CONNECTIVITY: portbindings.CONNECTIVITY_L2, portbindings.VIF_DETAILS_BOUND_DRIVERS: {'0': 'ovn'}, + portbindings.VIF_DETAILS_BRIDGE_NAME: 'br-int', + portbindings.OVS_DATAPATH_TYPE: 'netdev', } class TestPortBinding(base.TestOVNFunctionalBase): - def setUp(self): - super(TestPortBinding, self).setUp() + def setUp(self, **kwargs): + super().setUp(**kwargs) self.ovs_host = 'ovs-host' self.dpdk_host = 'dpdk-host' self.invalid_dpdk_host = 'invalid-host' self.insecure_host = 'insecure-host' self.smartnic_dpu_host = 'smartnic-dpu-host' self.smartnic_dpu_serial = 'fake-smartnic-dpu-serial' - self.add_fake_chassis(self.ovs_host) + self.add_fake_chassis( + self.ovs_host, + other_config={ovn_const.OVN_DATAPATH_TYPE: 'system'}) self.add_fake_chassis( self.dpdk_host, - other_config={'datapath-type': 'netdev', + other_config={ovn_const.OVN_DATAPATH_TYPE: 'netdev', 'iface-types': 'dummy,dummy-internal,dpdkvhostuser'}) self.add_fake_chassis( self.invalid_dpdk_host, - other_config={'datapath-type': 'netdev', + other_config={ovn_const.OVN_DATAPATH_TYPE: 'netdev', 'iface-types': 'dummy,dummy-internal,geneve,vxlan'}) self.add_fake_chassis( self.smartnic_dpu_host, other_config={ovn_const.OVN_CMS_OPTIONS: '{}={}'.format( ovn_const.CMS_OPT_CARD_SERIAL_NUMBER, - self.smartnic_dpu_serial)}) + self.smartnic_dpu_serial), + ovn_const.OVN_DATAPATH_TYPE: 'system'}) self.n1 = self._make_network(self.fmt, 'n1', True) res = self._create_subnet(self.fmt, self.n1['network']['id'], '10.0.0.0/24') @@ -151,9 +158,13 @@ def test_port_binding_create_port(self): self._verify_vif_details(port_id, self.dpdk_host, 'vhostuser', expected_vif_details) + expected_vif_details = copy.deepcopy(VHOSTUSER_VIF_DETAILS) + expected_vif_details.pop('vhostuser_mode') + expected_vif_details.pop('vhostuser_ovs_plug') + expected_vif_details[portbindings.CAP_PORT_FILTER] = True port_id = self._create_or_update_port(hostname=self.invalid_dpdk_host) self._verify_vif_details(port_id, self.invalid_dpdk_host, 'ovs', - OVS_VIF_DETAILS) + expected_vif_details) def test_port_binding_create_remote_managed_port(self): pci_vendor_info = 'fake-pci-vendor-info' @@ -205,8 +216,12 @@ def test_port_binding_update_port(self): port_id = self._create_or_update_port(port_id=port_id, hostname=self.invalid_dpdk_host) + expected_vif_details = copy.deepcopy(VHOSTUSER_VIF_DETAILS) + expected_vif_details.pop('vhostuser_mode') + expected_vif_details.pop('vhostuser_ovs_plug') + expected_vif_details[portbindings.CAP_PORT_FILTER] = True self._verify_vif_details(port_id, self.invalid_dpdk_host, 'ovs', - OVS_VIF_DETAILS) + expected_vif_details) def test_port_binding_update_remote_managed_port(self): port_id = self._create_or_update_port( diff --git a/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py b/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py index 18b14c44241..c0b11d8c417 100644 --- a/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py +++ b/neutron/tests/unit/api/rpc/agentnotifiers/test_dhcp_rpc_agent_api.py @@ -251,6 +251,7 @@ def test__notify_agents_with_router_interface_delete(self): payload = events.DBEventPayload( mock.Mock(), metadata={ 'port': {'id': 'foo_port_id', + 'fixed_ips': mock.ANY, 'network_id': 'foo_network_id'}}) self._test__notify_agents_with_function( lambda: self.notifier._after_router_interface_deleted( diff --git a/neutron/tests/unit/common/ovn/test_utils.py b/neutron/tests/unit/common/ovn/test_utils.py index 7b919f6e82b..8012abe3554 100644 --- a/neutron/tests/unit/common/ovn/test_utils.py +++ b/neutron/tests/unit/common/ovn/test_utils.py @@ -711,6 +711,11 @@ def test_valid_input_surplus_keys(self): {portbindings.VNIC_TYPE: portbindings.VNIC_DIRECT, constants.OVN_PORT_BINDING_PROFILE: binding_profile})) + def test_valid_input_no_binding_profile(self): + # Confirm that we treat a port without binding:profile as valid + self.assertEqual(utils.BPInfo({}, None, []), + utils.validate_and_get_data_from_binding_profile({})) + def test_unknown_profile_items_pruned(self): # Confirm that unknown profile items are pruned self.assertEqual( diff --git a/neutron/tests/unit/fake_resources.py b/neutron/tests/unit/fake_resources.py index 37d76cbbc59..c7d58710894 100644 --- a/neutron/tests/unit/fake_resources.py +++ b/neutron/tests/unit/fake_resources.py @@ -184,6 +184,7 @@ def __init__(self, **kwargs): self.get_extport_chassis_from_cms_options = mock.Mock(return_value=[]) self.is_col_present = mock.Mock() self.is_col_present.return_value = False + self.db_find = mock.MagicMock() self.db_set = mock.Mock() self.lookup = mock.MagicMock() self.chassis_list = mock.MagicMock() @@ -858,7 +859,8 @@ class FakeChassis(object): def create(attrs=None, az_list=None, chassis_as_gw=False, bridge_mappings=None, rp_bandwidths=None, rp_inventory_defaults=None, rp_hypervisors=None, - card_serial_number=None, chassis_as_extport=False): + card_serial_number=None, chassis_as_extport=False, + datapath_type=None): cms_opts = [] if az_list: cms_opts.append("%s=%s" % (ovn_const.CMS_OPT_AVAILABILITY_ZONES, @@ -903,6 +905,9 @@ def create(attrs=None, az_list=None, chassis_as_gw=False, if bridge_mappings: other_config['ovn-bridge-mappings'] = ','.join(bridge_mappings) + if datapath_type: + other_config[ovn_const.OVN_DATAPATH_TYPE] = datapath_type + chassis_attrs = { 'encaps': [], 'external_ids': '', diff --git a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_sriov_nic_agent.py b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_sriov_nic_agent.py index ede87aa439b..b0a4b783396 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_sriov_nic_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_sriov_nic_agent.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import collections import copy from unittest import mock @@ -460,6 +461,7 @@ def __init__(self): self.activated_bindings = set() self.conf = mock.Mock() self.conf.host = 'host1' + self.network_ports = collections.defaultdict(list) class TestSriovNicSwitchRpcCallbacks(base.BaseTestCase): @@ -528,6 +530,12 @@ def test_binding_activate(self): } kwargs = self._create_fake_bindings(fake_port, self.agent.conf.host) kwargs['context'] = self.context + + self.agent.network_ports['network_id'].append({ + 'port_id': fake_port['id'], + 'device': 'fake_device' + }) + self.sriov_rpc_callback.binding_activate(**kwargs) # Assert agent.activated_binding set contains the new binding self.assertIn((fake_port['mac_address'], @@ -538,10 +546,32 @@ def test_binding_activate_no_host(self): fake_port = self._create_fake_port() kwargs = self._create_fake_bindings(fake_port, 'other-host') kwargs['context'] = self.context + + self.agent.network_ports[self.agent.conf.host].append({ + 'port_id': fake_port['id'], + 'device': 'fake_device' + }) + self.sriov_rpc_callback.binding_activate(**kwargs) # Assert no bindings were added self.assertEqual(set(), self.agent.activated_bindings) + def test_binding_activate_port_not_in_network(self): + fake_port = self._create_fake_port() + kwargs = self._create_fake_bindings(fake_port, self.agent.conf.host) + kwargs['context'] = self.context + + self.agent.network_ports['network_id'] = [] + + with mock.patch.object(sriov_nic_agent.LOG, + 'warning') as mock_warning: + self.sriov_rpc_callback.binding_activate(**kwargs) + # Check that the warning message was logged + expected_msg = ( + "This port is not SRIOV, skip binding for port %s." + ) + mock_warning.assert_called_once_with(expected_msg, fake_port['id']) + def test_binding_deactivate(self): # binding_deactivate() basically does nothing # call it with both the agent's host and other host to cover diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 32323fe1e02..3f32152fe3b 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -46,6 +46,7 @@ from ovsdbapp.backend.ovs_idl import idlutils from webob import exc +from neutron.common import _constants as n_const from neutron.common import config from neutron.common.ovn import acl as ovn_acl from neutron.common.ovn import constants as ovn_const @@ -75,6 +76,7 @@ OVN_PROFILE = ovn_const.OVN_PORT_BINDING_PROFILE CLASS_PLACEMENT_REPORT = ('neutron.services.placement_report.plugin.' 'PlacementReportPlugin') +DEFAULT_DP_TYPE = 'system' # For testing, we define "system" as default. OvnRevNumberRow = collections.namedtuple( 'OvnRevNumberRow', ['created_at']) @@ -100,33 +102,47 @@ def setUp(self): self.mock_vp_parents = mock.patch.object( ovn_utils, 'get_virtual_port_parents', return_value=None).start() - def _add_chassis(self, nb_cfg, name=None): + def _add_chassis_private(self, nb_cfg, name=None): chassis_private = mock.Mock() chassis_private.nb_cfg = nb_cfg chassis_private.uuid = uuid.uuid4() chassis_private.name = name if name else str(uuid.uuid4()) return chassis_private - def _add_chassis_agent(self, nb_cfg, agent_type, chassis_private=None): - chassis_private = chassis_private or self._add_chassis(nb_cfg) + def _add_chassis(self, name, hostname, external_ids=None, + other_config=None): + external_ids = external_ids or {} + other_config = other_config or {} + return mock.Mock(name=name, hostname=hostname, + external_ids=external_ids, other_config=other_config) + + def _add_chassis_agent(self, nb_cfg, agent_type, chassis_private=None, + hostname=None): + chassis_private = chassis_private or self._add_chassis_private(nb_cfg) + hostname = hostname or chassis_private.name + '_host' if hasattr(chassis_private, 'nb_cfg_timestamp') and isinstance( chassis_private.nb_cfg_timestamp, mock.Mock): del chassis_private.nb_cfg_timestamp - chassis_private.external_ids = {} - chassis_private.other_config = {} + chassis_private.external_ids = { + ovn_const.OVN_AGENT_OVN_BRIDGE: n_const.DEFAULT_BR_INT, + ovn_const.OVN_DATAPATH_TYPE: DEFAULT_DP_TYPE, + } if agent_type == ovn_const.OVN_METADATA_AGENT: chassis_private.external_ids.update({ ovn_const.OVN_AGENT_METADATA_SB_CFG_KEY: nb_cfg, ovn_const.OVN_AGENT_METADATA_ID_KEY: str(uuid.uuid4())}) - chassis_private.chassis = [chassis_private] + chassis_private.chassis = [self._add_chassis(chassis_private.name, + hostname)] return neutron_agent.AgentCache().update(agent_type, chassis_private) - def _add_agent(self, name, nb_cfg_offset=0): + def _add_agent(self, name, nb_cfg_offset=0, hostname=None): + hostname = hostname or name + '_host' nb_cfg = 5 self.mech_driver.nb_ovn.nb_global.nb_cfg = nb_cfg + nb_cfg_offset - chassis = self._add_chassis(nb_cfg, name=name) + chassis_private = self._add_chassis_private(nb_cfg, name=name) return self._add_chassis_agent( - nb_cfg, ovn_const.OVN_CONTROLLER_AGENT, chassis) + nb_cfg, ovn_const.OVN_CONTROLLER_AGENT, + chassis_private=chassis_private, hostname=hostname) class TestOVNMechanismDriverBase(MechDriverSetupBase, @@ -1244,6 +1260,9 @@ def test_bind_port_host_not_alive(self): self._test_bind_port_failed([]) def _test_bind_port(self, fake_segments): + fake_chassis = fakes.FakeChassis.create(datapath_type=DEFAULT_DP_TYPE) + self.sb_ovn.db_find.return_value.execute.return_value = [ + {'other_config': fake_chassis.other_config}] fake_port = fakes.FakePort.create_one_port().info() fake_host = 'host' fake_port_context = fakes.FakePortContext( @@ -1252,12 +1271,18 @@ def _test_bind_port(self, fake_segments): neutron_agent.AgentCache().get_agents.assert_called_once_with( {'host': fake_host, 'agent_type': ovn_const.OVN_CONTROLLER_TYPES}) - fake_port_context.set_binding.assert_called_once_with( - fake_segments[0]['id'], - portbindings.VIF_TYPE_OVS, + vif_details = copy.deepcopy( self.mech_driver.vif_details[portbindings.VIF_TYPE_OVS]) + vif_details[ + portbindings.VIF_DETAILS_BRIDGE_NAME] = n_const.DEFAULT_BR_INT + vif_details[portbindings.OVS_DATAPATH_TYPE] = DEFAULT_DP_TYPE + fake_port_context.set_binding.assert_called_once_with( + fake_segments[0]['id'], portbindings.VIF_TYPE_OVS, vif_details) def _test_bind_port_sriov(self, fake_segments): + fake_chassis = fakes.FakeChassis.create(datapath_type=DEFAULT_DP_TYPE) + self.sb_ovn.db_find.return_value.execute.return_value = [ + {'other_config': fake_chassis.other_config}] fake_port = fakes.FakePort.create_one_port( attrs={'binding:vnic_type': 'direct', 'binding:profile': { @@ -1270,10 +1295,13 @@ def _test_bind_port_sriov(self, fake_segments): neutron_agent.AgentCache().get_agents.assert_called_once_with( {'host': fake_host, 'agent_type': ovn_const.OVN_CONTROLLER_TYPES}) - fake_port_context.set_binding.assert_called_once_with( - fake_segments[0]['id'], - portbindings.VIF_TYPE_OVS, + vif_details = copy.deepcopy( self.mech_driver.vif_details[portbindings.VIF_TYPE_OVS]) + vif_details[ + portbindings.VIF_DETAILS_BRIDGE_NAME] = n_const.DEFAULT_BR_INT + vif_details[portbindings.OVS_DATAPATH_TYPE] = DEFAULT_DP_TYPE + fake_port_context.set_binding.assert_called_once_with( + fake_segments[0]['id'], portbindings.VIF_TYPE_OVS, vif_details) def _test_bind_port_remote_managed(self, fake_segments): fake_serial = 'fake-serial' @@ -1292,6 +1320,9 @@ def _test_bind_port_remote_managed(self, fake_segments): attrs={'hostname': fake_smartnic_dpu}, card_serial_number=fake_serial) + fake_chassis = fakes.FakeChassis.create(datapath_type=DEFAULT_DP_TYPE) + self.sb_ovn.db_find.return_value.execute.return_value = [ + {'other_config': fake_chassis.other_config}] self.sb_ovn.get_chassis_by_card_serial_from_cms_options.\ return_value = ch_smartnic_dpu fake_host = 'host' @@ -1301,12 +1332,18 @@ def _test_bind_port_remote_managed(self, fake_segments): neutron_agent.AgentCache().get_agents.assert_called_once_with( {'host': fake_smartnic_dpu, 'agent_type': ovn_const.OVN_CONTROLLER_TYPES}) - fake_port_context.set_binding.assert_called_once_with( - fake_segments[0]['id'], - portbindings.VIF_TYPE_OVS, + vif_details = copy.deepcopy( self.mech_driver.vif_details[portbindings.VIF_TYPE_OVS]) + vif_details[ + portbindings.VIF_DETAILS_BRIDGE_NAME] = n_const.DEFAULT_BR_INT + vif_details[portbindings.OVS_DATAPATH_TYPE] = DEFAULT_DP_TYPE + fake_port_context.set_binding.assert_called_once_with( + fake_segments[0]['id'], portbindings.VIF_TYPE_OVS, vif_details) def test_bind_port_vdpa(self): + fake_chassis = fakes.FakeChassis.create(datapath_type=DEFAULT_DP_TYPE) + self.sb_ovn.db_find.return_value.execute.return_value = [ + {'other_config': fake_chassis.other_config}] segment_attrs = {'network_type': 'geneve', 'physical_network': None, 'segmentation_id': 1023} @@ -1323,10 +1360,13 @@ def test_bind_port_vdpa(self): neutron_agent.AgentCache().get_agents.assert_called_once_with( {'host': fake_host, 'agent_type': ovn_const.OVN_CONTROLLER_TYPES}) - fake_port_context.set_binding.assert_called_once_with( - fake_segments[0]['id'], - portbindings.VIF_TYPE_OVS, + vif_details = copy.deepcopy( self.mech_driver.vif_details[portbindings.VIF_TYPE_OVS]) + vif_details[ + portbindings.VIF_DETAILS_BRIDGE_NAME] = n_const.DEFAULT_BR_INT + vif_details[portbindings.OVS_DATAPATH_TYPE] = DEFAULT_DP_TYPE + fake_port_context.set_binding.assert_called_once_with( + fake_segments[0]['id'], portbindings.VIF_TYPE_OVS, vif_details) def test_bind_port_geneve(self): segment_attrs = {'network_type': 'geneve', @@ -2340,7 +2380,7 @@ def test_update_port_postcommit_revision_mismatch_not_after_live_migration( mock_notify_dhcp.assert_called_with(fake_port['id']) def test_agent_alive_true(self): - chassis_private = self._add_chassis(5) + chassis_private = self._add_chassis_private(5) for agent_type in (ovn_const.OVN_CONTROLLER_AGENT, ovn_const.OVN_METADATA_AGENT): self.mech_driver.nb_ovn.nb_global.nb_cfg = 5 @@ -2352,7 +2392,7 @@ def test_agent_alive_true_one_diff(self): # Agent should be reported as alive when the nb_cfg delta is 1 # even if the last update time was old enough. nb_cfg = 5 - chassis_private = self._add_chassis(nb_cfg) + chassis_private = self._add_chassis_private(nb_cfg) for agent_type in (ovn_const.OVN_CONTROLLER_AGENT, ovn_const.OVN_METADATA_AGENT): self.mech_driver.nb_ovn.nb_global.nb_cfg = nb_cfg + 1 @@ -2367,7 +2407,7 @@ def test_agent_alive_true_one_diff(self): def test_agent_alive_not_timed_out(self): nb_cfg = 3 - chassis_private = self._add_chassis(nb_cfg) + chassis_private = self._add_chassis_private(nb_cfg) for agent_type in (ovn_const.OVN_CONTROLLER_AGENT, ovn_const.OVN_METADATA_AGENT): self.mech_driver.nb_ovn.nb_global.nb_cfg = nb_cfg + 2 @@ -2378,7 +2418,7 @@ def test_agent_alive_not_timed_out(self): def test_agent_alive_timed_out(self): nb_cfg = 3 - chassis_private = self._add_chassis(nb_cfg) + chassis_private = self._add_chassis_private(nb_cfg) for agent_type in (ovn_const.OVN_CONTROLLER_AGENT, ovn_const.OVN_METADATA_AGENT): self.mech_driver.nb_ovn.nb_global.nb_cfg = nb_cfg + 2 @@ -2393,7 +2433,7 @@ def test_agent_alive_timed_out(self): def test_agent_with_nb_cfg_timestamp_timeout(self): nb_cfg = 3 - chassis_private = self._add_chassis(nb_cfg) + chassis_private = self._add_chassis_private(nb_cfg) self.mech_driver.nb_ovn.nb_global.nb_cfg = nb_cfg + 2 updated_at = (timeutils.utcnow_ts() - cfg.CONF.agent_down_time - 1 @@ -2407,7 +2447,7 @@ def test_agent_with_nb_cfg_timestamp_timeout(self): def test_agent_with_nb_cfg_timestamp_not_timeout(self): nb_cfg = 3 - chassis_private = self._add_chassis(nb_cfg) + chassis_private = self._add_chassis_private(nb_cfg) self.mech_driver.nb_ovn.nb_global.nb_cfg = nb_cfg + 2 updated_at = timeutils.utcnow_ts() * 1000 diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/test_db_migration.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/test_db_migration.py index ad7d56a39ec..41b0b490b05 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/test_db_migration.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/test_db_migration.py @@ -11,7 +11,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. - +import copy from unittest import mock from neutron_lib.api.definitions import portbindings as pb @@ -21,6 +21,7 @@ from neutron_lib import exceptions from oslo_utils import uuidutils +from neutron.common import _constants as n_const from neutron.db.models.plugins.ml2 import geneveallocation from neutron.db.models.plugins.ml2 import vxlanallocation from neutron.objects import ports as port_obj @@ -46,11 +47,12 @@ def _create_ml2_ovs_test_resources(self, vif_details_list): for vif_details in vif_details_list: port = self._make_port(self.fmt, network_id)['port'] - port_o = port_obj.PortBinding.get_object( - ctx, port_id=port['id'], host='') - port_o.vif_type = 'ovs' - port_o.vif_details = vif_details - port_o.update() + with db_api.CONTEXT_WRITER.using(ctx): + port_o = port_obj.PortBinding.get_object( + ctx, port_id=port['id'], host='') + port_o.vif_type = 'ovs' + port_o.vif_details = vif_details + port_o.update() for i in range(1, 4): port = self._make_port(self.fmt, network_id)['port'] @@ -150,14 +152,10 @@ def test_db_migration(self): {"foo": "bar"}, {}, ] - expected_vif_details = [ - {pb.CAP_PORT_FILTER: "true", - pb.OVS_HYBRID_PLUG: "true", - pb.VIF_DETAILS_CONNECTIVITY: pb.CONNECTIVITY_L2}, - {pb.CAP_PORT_FILTER: "true"}, - {"foo": "bar"}, - {}, - ] + expected_vif_details = copy.deepcopy(vif_details_list) + for vif_detail in expected_vif_details: + vif_detail[pb.VIF_DETAILS_BRIDGE_NAME] = n_const.DEFAULT_BR_INT + expected_vif_details.append({}) self._create_ml2_ovs_test_resources(vif_details_list) db_migration.migrate_neutron_database_to_ovn() diff --git a/releasenotes/notes/ovn-vif-details-bridge-name-and-datapath-type-d2bd5b438118355f.yaml b/releasenotes/notes/ovn-vif-details-bridge-name-and-datapath-type-d2bd5b438118355f.yaml new file mode 100644 index 00000000000..365c436477d --- /dev/null +++ b/releasenotes/notes/ovn-vif-details-bridge-name-and-datapath-type-d2bd5b438118355f.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + [`bug 2045889 `_] + The ports bound to ML2/OVN now contain the OVS bridge name and datapath + type in the VIF details dictionary. NOTE: in the ML2/OVS to ML2/OVN + migration, the local host OVN bridge (integration bridge) per port is not + known; "br-int" will be used by default (that value is rarely changed).