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

Conflicts:
    neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py
    neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py

Change-Id: I6cf1167d556f0c2c2aa2013f05c809648020b377
(cherry picked from commit 3c93da7bdf35c3be6081384b7f6d9a4ce13510d3)

7 files changed, 71 insertions, 39 deletions

diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py
index ef8e9de221..e22bb3b357 100644
--- a/neutron/common/ovn/utils.py
+++ b/neutron/common/ovn/utils.py
@@ -406,6 +406,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 92c27b496f..77ed69f372 100644
--- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py
+++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py
@@ -807,6 +807,39 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
                     txn.add(cmd)
         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 c5c7b5cd54..77940fba94 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 07878f852d..73d5a31ba7 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
@@ -1067,8 +1067,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 dc0539d12d..c44d8a9d7e 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 9015e7ba30..b1cd653a48 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
@@ -589,3 +589,22 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight,
             mock.call('Logical_Router_Port', 'lrp-port1', ('options', opt))]
         self.fake_ovn_client._nb_idl.db_set.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 f88a6c6485..d8a64665ba 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
@@ -90,6 +90,8 @@ class MechDriverSetupBase:
         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()
@@ -181,9 +183,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)
@@ -211,12 +210,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(
@@ -2448,9 +2441,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,
@@ -2762,9 +2752,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
 
@@ -3467,9 +3455,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(
@@ -3836,9 +3821,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
@@ -4003,11 +3986,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',