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',