From 6f0da8a5bef905235f4e5a1cd73c6f56b301af6c Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Wed, 23 Mar 2022 13:34:14 -0400 Subject: [PATCH 1/6] ovn migration: Turn validations off by default The validation is intended mostly for tests and don't make much sense when running the migration in production because likely there are already running workloads. This patch changes the default to False so migration validation must be explicitly asked for. Change-Id: I5470f61a5e0b55bf682526208c3f57dc0ca6ffd5 Signed-off-by: Jakub Libosvar (cherry picked from commit 0baf8841eea402b7b8505ee685b1ac1a3f346df5) --- doc/source/ovn/migration.rst | 2 +- ...change-migration-validation-b030b02c5e1acd3d.yaml | 12 ++++++++++++ .../tripleo_environment/ovn_migration.sh | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/change-migration-validation-b030b02c5e1acd3d.yaml diff --git a/doc/source/ovn/migration.rst b/doc/source/ovn/migration.rst index 14ad24e89f7..577afc24f99 100644 --- a/doc/source/ovn/migration.rst +++ b/doc/source/ovn/migration.rst @@ -136,7 +136,7 @@ Perform the following steps in the undercloud * VALIDATE_MIGRATION - Create migration resources to validate the migration. The migration script, before starting the migration, boot a server and validates that the server is reachable after the migration. - Default: True. + Default: False * SERVER_USER_NAME - User name to use for logging into the migration instances. diff --git a/releasenotes/notes/change-migration-validation-b030b02c5e1acd3d.yaml b/releasenotes/notes/change-migration-validation-b030b02c5e1acd3d.yaml new file mode 100644 index 00000000000..94aa5e319c3 --- /dev/null +++ b/releasenotes/notes/change-migration-validation-b030b02c5e1acd3d.yaml @@ -0,0 +1,12 @@ +--- +other: + - | + The OVN migration performs validation by default. This validation means an + instance is spawned and is tested by simple ping after the migration is + finished. Also it tries to create new workload post migration. This is + useful for very simple scenarios when migration is tested but is not + really useful in production since likely the production envrionments + already have running workloads. It makes more sense to require the + validation explicitly rather than implicitly run it as the migration + is mostly intended for production. The VALIDATE_MIGRATION now defaults to + False and needs to be changed to True if validation upon request. diff --git a/tools/ovn_migration/tripleo_environment/ovn_migration.sh b/tools/ovn_migration/tripleo_environment/ovn_migration.sh index 19194d7c1b2..80944f1a66b 100644 --- a/tools/ovn_migration/tripleo_environment/ovn_migration.sh +++ b/tools/ovn_migration/tripleo_environment/ovn_migration.sh @@ -39,7 +39,7 @@ LANG=C : ${IMAGE_NAME:=cirros} : ${FLAVOR_NAME:=ovn-migration} : ${SERVER_USER_NAME:=cirros} -: ${VALIDATE_MIGRATION:=True} +: ${VALIDATE_MIGRATION:=False} : ${DHCP_RENEWAL_TIME:=30} : ${CREATE_BACKUP:=True} : ${BACKUP_MIGRATION_IP:=192.168.24.1} # TODO: Document this new var From 9ee1cae86a394af7ed8c19a8e26e498b96e086db Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Tue, 28 Feb 2023 18:02:27 +0900 Subject: [PATCH 2/6] Add missing osprofiler options osprofiler was integrated to Neutron a while ago[1] but the options for this library have not been added to neutron.conf properly. This ensures the options are rendered by oslo-config-generator. [1] 9a43f58f4df85adc2029c33ba000ca17b746a6eb Change-Id: Ice1b3f701ac244e17d855484263199f8a0b8310b (cherry picked from commit 289ae97c1ccaed72938d4294aa003c0db3b82f70) --- etc/oslo-config-generator/neutron.conf | 1 + 1 file changed, 1 insertion(+) diff --git a/etc/oslo-config-generator/neutron.conf b/etc/oslo-config-generator/neutron.conf index 9f3212badfe..2ab0a0e7be9 100644 --- a/etc/oslo-config-generator/neutron.conf +++ b/etc/oslo-config-generator/neutron.conf @@ -25,3 +25,4 @@ namespace = oslo.service.service namespace = oslo.service.sslutils namespace = oslo.service.wsgi namespace = keystonemiddleware.auth_token +namespace = osprofiler From 61ff4a1cc11c415269068f96f3238620527adb57 Mon Sep 17 00:00:00 2001 From: James Denton Date: Mon, 27 Feb 2023 14:44:08 -0600 Subject: [PATCH 3/6] Apply Ironic's server-ip-address as TFTP next-server This patch uses existing DHCP option 255 (server-ip-address) provided by Ironic and applies it as next-server for the purpose of provisioning a baremetal server with OVN DHCP. Related-Bug: #2007167 Change-Id: I59038639a8411c11c5fb8b366d9c858ef3db4f70 (cherry picked from commit dbfc18d1fa122140074d5c960c8440189b386fb8) --- neutron/common/ovn/constants.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index d8abb74a0b3..93e9035c7d7 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -166,7 +166,8 @@ '119': 'domain_search_list', '252': 'wpad', '210': 'path_prefix', - '150': 'tftp_server_address'}, + '150': 'tftp_server_address', + '255': 'next_server'}, 6: {'server-id': 'server_id', 'dns-server': 'dns_server', 'domain-search': 'domain_search', From 1f9f77e4a27ac41084037df625fef124fb19163c Mon Sep 17 00:00:00 2001 From: Anton Kurbatov Date: Thu, 23 Feb 2023 15:07:11 +0000 Subject: [PATCH 4/6] Prevent router_ha_interface port from being removed via API If someone removes the port with device owner router_ha_interface, then we can get unexpected router behavior like doubling arp response packets. This patch prohibits removing such a port. Closes-Bug: #2008270 Change-Id: Ief031801c1a3e3dd64e6cbf65e27f04f2bef9cba (cherry picked from commit e68e4162cebfa8e5d8f70d4220119d5ca54666bf) --- neutron/db/l3_hamode_db.py | 4 ++++ neutron/tests/unit/db/test_l3_hamode_db.py | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py index 1209fc1e1bb..048a92515af 100644 --- a/neutron/db/l3_hamode_db.py +++ b/neutron/db/l3_hamode_db.py @@ -69,6 +69,10 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin, router_az_db.RouterAvailabilityZoneMixin): """Mixin class to add high availability capability to routers.""" + router_device_owners = ( + l3_dvr_db.L3_NAT_with_dvr_db_mixin.router_device_owners + + (constants.DEVICE_OWNER_ROUTER_HA_INTF, )) + def _verify_configuration(self): self.ha_cidr = cfg.CONF.l3_ha_net_cidr try: diff --git a/neutron/tests/unit/db/test_l3_hamode_db.py b/neutron/tests/unit/db/test_l3_hamode_db.py index 57e06c042b0..7e9dd9b72fd 100644 --- a/neutron/tests/unit/db/test_l3_hamode_db.py +++ b/neutron/tests/unit/db/test_l3_hamode_db.py @@ -667,6 +667,18 @@ def test_update_router_ha_interface_port_ip_not_allow(self): self.admin_ctx, ports[0]['id'], port) + def test_delete_router_ha_interface_port(self): + router = self._create_router() + network = self.plugin.get_ha_network(self.admin_ctx, + router['tenant_id']) + binding = self.plugin.add_ha_port( + self.admin_ctx, router['id'], network.network_id, + router['tenant_id']) + + self.assertRaises(n_exc.ServicePortInUse, + self.core_plugin.delete_port, + self.admin_ctx, binding.port_id) + def test_create_ha_network_tenant_binding_raises_duplicate(self): router = self._create_router() network = self.plugin.get_ha_network(self.admin_ctx, From 334f7733f5b655b4f06d101752c2c192640a1f14 Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Fri, 20 Jan 2023 12:16:06 +0100 Subject: [PATCH 5/6] [OVN] Ensure traffic for provider vlan networks is not tunneled This patch adds an extra checking to ensure the "reside-on-redirect-chassis" is set to true for the logical router port associated to vlan provider network despite having the "ovn_distributed_floating_ip" enabled or not. This is needed as there is an OVN bug [1] making it not work as expected. Note setting this to true has implications as the traffic will be centrallized (but not tunneled) through the node with the gateway port. The expected behavior of this flag, once [1] is fixed is: - reside-on-redirect-chassis flag to False: means traffic goes tunneled to the controller with the gateway port. Means it requires extra MTU reduction to work. - reside-on-redirect-chassis flag to True: means traffic is not tunneled to the controller with the gateway port, but the traffic is centralized through the controller with the gateway port. Thus it does not require extra MTU reduction. - reside-on-redirect-chassis to False, but with ovn-chassis-mac-mappings configured: means the traffic is fully distributed and it is not being tunneled, nor sent, through the controller with the gateway port. This is the preferred option as it does not require MTU reduction and it avoids the extra hop. However it is not working as expected, therefore the fallback to set reside-on-redirect-chassis to True. [1] https://bugzilla.redhat.com/show_bug.cgi?id=2162756 Closes-Bug: #2003455 Change-Id: I662cb30c842e54bb9f7dabac5519283aa7c7f8d0 (cherry picked from commit acb809eea422f417d4bfb2d46918839d7d379e4c) --- neutron/common/ovn/utils.py | 5 +++++ .../ovn/mech_driver/ovsdb/maintenance.py | 5 ++++- .../ovn/mech_driver/ovsdb/ovn_client.py | 12 ++++++++---- .../notes/bug-2003455-b502cc637427560e.yaml | 19 +++++++++++++++++++ 4 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/bug-2003455-b502cc637427560e.yaml diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 9031b501fe3..467f0311bdd 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -21,6 +21,7 @@ from neutron_lib.api.definitions import l3 from neutron_lib.api.definitions import port_security as psec from neutron_lib.api.definitions import portbindings +from neutron_lib.api.definitions import provider_net from neutron_lib.api import validators from neutron_lib import constants as const from neutron_lib import context as n_context @@ -610,6 +611,10 @@ def is_gateway_chassis_invalid(chassis_name, gw_chassis, def is_provider_network(network): + return network.get(provider_net.PHYSICAL_NETWORK, False) + + +def is_external_network(network): return network.get(external_net.EXTERNAL, False) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index a25e75bcb4b..a6c1f1c628e 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -831,7 +831,10 @@ def check_vlan_distributed_ports(self): # Get router ports belonging to VLAN networks vlan_nets = self._ovn_client._plugin.get_networks( context, {pnet.NETWORK_TYPE: [n_const.TYPE_VLAN]}) - vlan_net_ids = [vn['id'] for vn in vlan_nets] + # FIXME(ltomasbo): Once Bugzilla 2162756 is fixed the + # is_provider_network check should be removed + vlan_net_ids = [vn['id'] for vn in vlan_nets + if not utils.is_provider_network(vn)] router_ports = self._ovn_client._plugin.get_ports( context, {'network_id': vlan_net_ids, 'device_owner': n_const.ROUTER_PORT_OWNERS}) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 71b8d970848..a62324135b7 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -1247,7 +1247,7 @@ def _get_nets_and_ipv6_ra_confs_for_router_port(self, context, port): # leak the RAs generated for the tenant networks via the # provider network ipv6_ra_configs['send_periodic'] = 'true' - if is_gw_port and utils.is_provider_network(net): + if is_gw_port and utils.is_external_network(net): ipv6_ra_configs['send_periodic'] = 'false' ipv6_ra_configs['mtu'] = str(net['mtu']) @@ -1559,9 +1559,12 @@ def _gen_router_port_options(self, port, network=None): # logical router port is centralized in the chassis hosting the # distributed gateway port. # https://github.com/openvswitch/ovs/commit/85706c34d53d4810f54bec1de662392a3c06a996 + # FIXME(ltomasbo): Once Bugzilla 2162756 is fixed the + # is_provider_network check should be removed if network.get(pnet.NETWORK_TYPE) == const.TYPE_VLAN: options[ovn_const.LRP_OPTIONS_RESIDE_REDIR_CH] = ( - 'false' if ovn_conf.is_ovn_distributed_floating_ip() + 'false' if (ovn_conf.is_ovn_distributed_floating_ip() and + not utils.is_provider_network(network)) else 'true') is_gw_port = const.DEVICE_OWNER_ROUTER_GW == port.get( @@ -1976,8 +1979,9 @@ def update_network(self, context, network, original_network=None): for subnet in subnets: self.update_subnet(context, subnet, network, txn) - if utils.is_provider_network(network): - # make sure to use admin context as this is a providernet + if utils.is_external_network(network): + # make sure to use admin context as this is a external + # network self.set_gateway_mtu(n_context.get_admin_context(), network, txn) diff --git a/releasenotes/notes/bug-2003455-b502cc637427560e.yaml b/releasenotes/notes/bug-2003455-b502cc637427560e.yaml new file mode 100644 index 00000000000..2e89cf055f2 --- /dev/null +++ b/releasenotes/notes/bug-2003455-b502cc637427560e.yaml @@ -0,0 +1,19 @@ +--- +fixes: + - | + [`bug 2003455 `_] + It is added an extra checking to ensure the "reside-on-redirect-chassis" + is set to true for the logical router port associated to vlan provider + network despite having the "ovn_distributed_floating_ip" enabled or not. + This is needed as there is an OVN bug + (https://bugzilla.redhat.com/show_bug.cgi?id=2162756) making it not work + as expected. Until that is fixed, we need these workaround + that makes the traffic centrallized, but not tunneled, through the node + with the gateway port, thus avoiding MTU issues. +issues: + - | + Until the OVN bug (https://bugzilla.redhat.com/show_bug.cgi?id=2162756) + is fixed, setting the "reside-on-redirect-chassis" to true for the logical + router port associated to vlan provider network is needed. This workaround + makes the traffic centrallized, but not tunneled, through the node + with the gateway port, thus avoiding MTU issues. From f5815bcca54daec2104d70d531456ff8c9c6e1fa Mon Sep 17 00:00:00 2001 From: Miro Tomaska Date: Fri, 17 Feb 2023 18:28:48 -0600 Subject: [PATCH 6/6] Make retrieval of port mac column safe This should not happen in the real world but its safer to check if a port MAC column is not empty before trying to access it. Trivial-Fix Change-Id: Ie3c5151a8f7c6a240a5f3240d4e7fb58ea43e9c1 (cherry picked from commit 51005388b8a7c1445a1e1bfdaeed3159acaccef2) --- neutron/agent/ovn/metadata/agent.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py index b90fed0fdb9..950cd5ab2a0 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -421,6 +421,10 @@ def _ensure_datapath_checksum(self, namespace): def _get_port_ips(self, port): # Retrieve IPs from the port mac column which is in form # [" ... "] + if not port.mac: + LOG.warning("Port %s MAC column is empty, cannot retrieve IP " + "addresses", port.uuid) + return [] mac_field_attrs = port.mac[0].split() ips = mac_field_attrs[1:] if not ips: