Merge "Set "type=virtual" for OVN LSP with parent ports"

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:

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):
 

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
 

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)]

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)

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)

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()
@@ -176,9 +178,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)
@@ -206,12 +205,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(
@@ -2479,9 +2472,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,
@@ -2793,9 +2783,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
 
@@ -3498,9 +3486,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(
@@ -3867,9 +3852,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
@@ -4034,11 +4017,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',