From 3c93da7bdf35c3be6081384b7f6d9a4ce13510d3 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Sat, 14 May 2022 23:54:10 +0000 Subject: [PATCH] Set "type=virtual" for OVN LSP with parent ports This is a follow-up of [1]. Before this patch, any virtual logical switch port that was updated and the "device_owner" was not and empty string, had its type set to '' (empty string). This maintenance task, that is executed only once, lists all logical switch ports, checks the presence or not of virtual parents and sets the type to "virtual" if needed. Related-Bug: #1973276 [1]https://review.opendev.org/c/openstack/neutron/+/841711 Change-Id: I6cf1167d556f0c2c2aa2013f05c809648020b377 --- neutron/common/ovn/utils.py | 7 ++++ .../ovn/mech_driver/ovsdb/maintenance.py | 33 +++++++++++++++++++ .../ovn/mech_driver/ovsdb/ovn_client.py | 10 ++---- .../tests/unit/db/test_db_base_plugin_v2.py | 5 ++- .../unit/extensions/test_portsecurity.py | 5 ++- .../ovn/mech_driver/ovsdb/test_maintenance.py | 19 +++++++++++ .../ovn/mech_driver/test_mech_driver.py | 31 ++++------------- 7 files changed, 71 insertions(+), 39 deletions(-) diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index d1d5b3e217b..3fd4f05b3e5 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -422,6 +422,13 @@ def get_ovn_port_addresses(ovn_port): return list(set(addresses + port_security)) +def get_virtual_port_parents(nb_idl, virtual_ip, network_id, port_id): + ls = nb_idl.ls_get(ovn_name(network_id)).execute(check_error=True) + return [lsp.name for lsp in ls.ports + if lsp.name != port_id and + virtual_ip in get_ovn_port_addresses(lsp)] + + def sort_ips_by_version(addresses): ip_map = {'ip4': [], 'ip6': []} for addr in addresses: 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 69c5bb18b04..9df613eff01 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -783,6 +783,39 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): raise periodics.NeverAgain() + # TODO(ralonsoh): Remove this in the Z+4 cycle + @periodics.periodic(spacing=600, run_immediately=True) + def update_port_virtual_type(self): + """Set type=virtual to those ports with parents + Before LP#1973276, any virtual port with "device_owner" defined, lost + its type=virtual. This task restores the type for those ports updated + before the fix https://review.opendev.org/c/openstack/neutron/+/841711. + """ + if not self.has_lock: + return + + context = n_context.get_admin_context() + cmds = [] + for lsp in self._nb_idl.lsp_list().execute(check_error=True): + if lsp.type != '': + continue + + port = self._ovn_client._plugin.get_port(context, lsp.name) + for ip in port.get('fixed_ips', []): + if utils.get_virtual_port_parents( + self._nb_idl, ip['ip_address'], port['network_id'], + port['id']): + cmds.append(self._nb_idl.db_set( + 'Logical_Switch_Port', lsp.uuid, + ('type', ovn_const.LSP_TYPE_VIRTUAL))) + break + + if cmds: + with self._nb_idl.transaction(check_error=True) as txn: + for cmd in cmds: + txn.add(cmd) + raise periodics.NeverAgain() + class HashRingHealthCheckPeriodics(object): 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 f52b532c297..df7b52d1f1d 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 @@ -215,13 +215,6 @@ class OVNClient(object): external_ids=subnet_dhcp_options['external_ids']) return {'cmd': add_dhcp_opts_cmd} - def get_virtual_port_parents(self, virtual_ip, port): - ls = self._nb_idl.ls_get(utils.ovn_name(port['network_id'])).execute( - check_error=True) - return [lsp.name for lsp in ls.ports - if lsp.name != port['id'] and - virtual_ip in utils.get_ovn_port_addresses(lsp)] - def determine_bind_host(self, port, port_context=None): """Determine which host the port should be bound to. @@ -299,7 +292,8 @@ class OVNClient(object): subnet['cidr'].split('/')[1]) # Check if the port being created is a virtual port - parents = self.get_virtual_port_parents(ip_addr, port) + parents = utils.get_virtual_port_parents( + self._nb_idl, ip_addr, port['network_id'], port['id']) if not parents: continue diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index ee7a6f845e8..f5096e331e5 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -50,6 +50,7 @@ from neutron.api import api_common from neutron.api import extensions from neutron.api.v2 import router from neutron.common import ipv6_utils +from neutron.common.ovn import utils as ovn_utils from neutron.common import test_lib from neutron.common import utils from neutron.conf import policies @@ -63,7 +64,6 @@ from neutron.ipam.drivers.neutrondb_ipam import driver as ipam_driver from neutron.ipam import exceptions as ipam_exc from neutron.objects import network as network_obj from neutron.objects import router as l3_obj -from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client from neutron import policy from neutron import quota from neutron.quota import resource_registry @@ -1075,8 +1075,7 @@ class TestPortsV2(NeutronDbPluginV2TestCase): def setUp(self, **kwargs): super().setUp(**kwargs) self.mock_vp_parents = mock.patch.object( - ovn_client.OVNClient, 'get_virtual_port_parents', - return_value=None).start() + ovn_utils, 'get_virtual_port_parents', return_value=None).start() def test_create_port_json(self): keys = [('admin_state_up', True), ('status', self.port_create_status)] diff --git a/neutron/tests/unit/extensions/test_portsecurity.py b/neutron/tests/unit/extensions/test_portsecurity.py index dc0539d12da..c44d8a9d7ed 100644 --- a/neutron/tests/unit/extensions/test_portsecurity.py +++ b/neutron/tests/unit/extensions/test_portsecurity.py @@ -25,11 +25,11 @@ from neutron_lib.exceptions import port_security as psec_exc from neutron_lib.plugins import directory from webob import exc +from neutron.common.ovn import utils as ovn_utils from neutron.db import db_base_plugin_v2 from neutron.db import portsecurity_db from neutron.db import securitygroups_db from neutron.extensions import securitygroup as ext_sg -from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client from neutron import quota from neutron.tests.unit.db import test_db_base_plugin_v2 from neutron.tests.unit.extensions import test_securitygroup @@ -188,8 +188,7 @@ class TestPortSecurity(PortSecurityDBTestCase): self.mock_quota_make_res = make_res.start() self.mock_quota_commit_res = commit_res.start() self.mock_vp_parents = mock.patch.object( - ovn_client.OVNClient, 'get_virtual_port_parents', - return_value=None).start() + ovn_utils, 'get_virtual_port_parents', return_value=None).start() def test_create_network_with_portsecurity_mac(self): res = self._create_network('json', 'net1', True) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 9c3b307c2cc..54858259686 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -686,3 +686,22 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, self.fake_ovn_client._nb_idl.set_lswitch_port.assert_has_calls( expected_calls) + + @mock.patch.object(utils, 'get_virtual_port_parents', + return_value=[mock.ANY]) + def test_update_port_virtual_type(self, *args): + nb_idl = self.fake_ovn_client._nb_idl + lsp0 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'lsp0', 'type': ''}) + lsp1 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'lsp1', 'type': constants.LSP_TYPE_VIRTUAL}) + port0 = {'fixed_ips': [{'ip_address': mock.ANY}], + 'network_id': mock.ANY, 'id': mock.ANY} + nb_idl.lsp_list.return_value.execute.return_value = (lsp0, lsp1) + self.fake_ovn_client._plugin.get_port.return_value = port0 + + self.assertRaises( + periodics.NeverAgain, self.periodic.update_port_virtual_type) + expected_calls = [mock.call('Logical_Switch_Port', lsp0.uuid, + ('type', constants.LSP_TYPE_VIRTUAL))] + nb_idl.db_set.assert_has_calls(expected_calls) 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 92442c65929..732ae5541c6 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 @@ -93,6 +93,8 @@ class MechDriverSetupBase(abc.ABC): agent1 = self._add_agent('agent1') neutron_agent.AgentCache().get_agents = mock.Mock() neutron_agent.AgentCache().get_agents.return_value = [agent1] + 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): chassis_private = mock.Mock() @@ -188,9 +190,6 @@ class TestOVNMechanismDriverBase(MechDriverSetupBase, p = mock.patch.object(ovn_revision_numbers_db, 'bump_revision') p.start() self.addCleanup(p.stop) - self.mock_vp_parents = mock.patch.object( - ovn_client.OVNClient, 'get_virtual_port_parents', - return_value=None).start() def test_delete_mac_binding_entries(self): self.config(group='ovn', ovn_sb_private_key=None) @@ -218,12 +217,6 @@ class TestOVNMechanismDriverBase(MechDriverSetupBase, class TestOVNMechanismDriver(TestOVNMechanismDriverBase): - def setUp(self): - super().setUp() - self.mock_vp_parents = mock.patch.object( - ovn_client.OVNClient, 'get_virtual_port_parents', - return_value=None).start() - @mock.patch.object(ovsdb_monitor.OvnInitPGNbIdl, 'from_server') @mock.patch.object(ovsdb_monitor, 'short_living_ovsdb_api') def test__create_neutron_pg_drop_non_existing( @@ -2483,9 +2476,6 @@ class OVNMechanismDriverTestCase(MechDriverSetupBase, p = mock.patch.object(ovn_utils, 'get_revision_number', return_value=1) p.start() self.addCleanup(p.stop) - self.mock_vp_parents = mock.patch.object( - ovn_client.OVNClient, 'get_virtual_port_parents', - return_value=None).start() class TestOVNMechanismDriverBasicGet(test_plugin.TestMl2BasicGet, @@ -2797,9 +2787,7 @@ class TestOVNMechanismDriverSegment(MechDriverSetupBase, segment_id=self.seg_2['id']) as subnet: self.sub_2 = subnet - @mock.patch.object(ovn_client.OVNClient, 'get_virtual_port_parents', - return_value=[]) - def test_create_segments_subnet_metadata_ip_allocation(self, *args): + def test_create_segments_subnet_metadata_ip_allocation(self): self._test_segments_helper() ovn_nb_api = self.mech_driver.nb_ovn @@ -3502,9 +3490,6 @@ class TestOVNMechanismDriverSecurityGroup(MechDriverSetupBase, super(TestOVNMechanismDriverSecurityGroup, self).setUp() self.ctx = context.get_admin_context() revision_plugin.RevisionPlugin() - self.mock_vp_parents = mock.patch.object( - ovn_client.OVNClient, 'get_virtual_port_parents', - return_value=None).start() def _delete_default_sg_rules(self, security_group_id): res = self._list( @@ -3871,9 +3856,7 @@ class TestOVNMechanismDriverMetadataPort(MechDriverSetupBase, with self.network(): self.assertEqual(0, self.nb_ovn.create_lswitch_port.call_count) - @mock.patch.object(ovn_client.OVNClient, 'get_virtual_port_parents', - return_value=[]) - def test_metadata_ip_on_subnet_create(self, *args): + def test_metadata_ip_on_subnet_create(self): """Check metadata port update. Check that the metadata port is updated with a new IP address when a @@ -4038,11 +4021,9 @@ class TestOVNVVirtualPort(OVNMechanismDriverTestCase): '10.0.0.1', '10.0.0.0/24')['subnet'] @mock.patch.object(ovn_client.OVNClient, 'determine_bind_host') - @mock.patch.object(ovn_client.OVNClient, 'get_virtual_port_parents') - def test_create_port_with_virtual_type_and_options( - self, mock_get_parents, mock_determine_bind_host): + def test_create_port_with_virtual_type_and_options(self, *args): fake_parents = ['parent-0', 'parent-1'] - mock_get_parents.return_value = fake_parents + self.mock_vp_parents.return_value = fake_parents for device_owner in ('', 'myVIPowner'): port = {'id': 'virt-port', 'mac_address': '00:00:00:00:00:00',