Merge "[OVN] Add missing LSP device_owner info in trunk driver" into stable/yoga

Author: Zuul

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py

@@ -40,16 +40,23 @@ def create_lswitch_port(self, lport_name, lswitch_name, may_exist=True,
         """
 
     @abc.abstractmethod
-    def set_lswitch_port(self, lport_name, if_exists=True, **columns):
+    def set_lswitch_port(self, lport_name, external_ids_update=None,
+                         if_exists=True, **columns):
         """Create a command to set OVN logical switch port fields
 
         :param lport_name:    The name of the lport
         :type lport_name:     string
+        :param external_ids_update: Dictionary of keys to be updated
+                              individually in the external IDs dictionary.
+                              If "external_ids" is defined in "columns",
+                              "external_ids" will override any
+                              "external_ids_update" value.
+        :type external_ids_update: dictionary
+        :param if_exists:     Do not fail if lport does not exist
+        :type if_exists:      bool
         :param columns:       Dictionary of port columns
                               Supported columns: macs, external_ids,
                                                  parent_name, tag, enabled
-        :param if_exists:     Do not fail if lport does not exist
-        :type if_exists:      bool
         :type columns:        dictionary
         :returns:             :class:`Command` with no result
         """

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py

@@ -148,9 +148,10 @@ def post_commit(self, txn):
 
 
 class SetLSwitchPortCommand(command.BaseCommand):
-    def __init__(self, api, lport, if_exists, **columns):
+    def __init__(self, api, lport, external_ids_update, if_exists, **columns):
         super(SetLSwitchPortCommand, self).__init__(api)
         self.lport = lport
+        self.external_ids_update = external_ids_update
         self.columns = columns
         self.if_exists = if_exists
 
@@ -195,6 +196,12 @@ def run_idl(self, txn):
         for uuid in cur_port_dhcp_opts - new_port_dhcp_opts:
             self.api._tables['DHCP_Options'].rows[uuid].delete()
 
+        external_ids_update = self.external_ids_update or {}
+        external_ids = getattr(port, 'external_ids', {})
+        for k, v in external_ids_update.items():
+            external_ids[k] = v
+        port.external_ids = external_ids
+
         for col, val in self.columns.items():
             setattr(port, col, val)
 

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py

@@ -283,8 +283,9 @@ def create_lswitch_port(self, lport_name, lswitch_name, may_exist=True,
         return cmd.AddLSwitchPortCommand(self, lport_name, lswitch_name,
                                          may_exist, **columns)
 
-    def set_lswitch_port(self, lport_name, if_exists=True, **columns):
-        return cmd.SetLSwitchPortCommand(self, lport_name,
+    def set_lswitch_port(self, lport_name, external_ids_update=None,
+                         if_exists=True, **columns):
+        return cmd.SetLSwitchPortCommand(self, lport_name, external_ids_update,
                                          if_exists, **columns)
 
     def delete_lswitch_port(self, lport_name=None, lswitch_name=None,

neutron/services/trunk/drivers/ovn/trunk_driver.py

@@ -21,7 +21,7 @@
 from oslo_config import cfg
 from oslo_log import log
 
-from neutron.common.ovn.constants import OVN_ML2_MECH_DRIVER_NAME
+from neutron.common.ovn import constants as ovn_const
 from neutron.objects import ports as port_obj
 from neutron.services.trunk.drivers import base as trunk_base
 
@@ -94,10 +94,13 @@ def _set_binding_profile(self, context, subport, parent_port,
             LOG.debug("Port not found while trying to set "
                       "binding_profile: %s", subport.port_id)
             return
+        ext_ids = {ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY: db_port.device_owner}
         ovn_txn.add(self.plugin_driver.nb_ovn.set_lswitch_port(
                     lport_name=subport.port_id,
                     parent_name=parent_port,
-                    tag=subport.segmentation_id))
+                    tag=subport.segmentation_id,
+                    external_ids_update=ext_ids,
+                    ))
         LOG.debug("Done setting parent %s for subport %s",
                   parent_port, subport.port_id)
 
@@ -128,11 +131,14 @@ def _unset_binding_profile(self, context, subport, ovn_txn):
             LOG.debug("Port not found while trying to unset "
                       "binding_profile: %s", subport.port_id)
             return
+        ext_ids = {ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY: db_port.device_owner}
         ovn_txn.add(self.plugin_driver.nb_ovn.set_lswitch_port(
                     lport_name=subport.port_id,
                     parent_name=[],
                     up=False,
-                    tag=[]))
+                    tag=[],
+                    external_ids_update=ext_ids,
+                    ))
         LOG.debug("Done unsetting parent for subport %s", subport.port_id)
 
     def trunk_created(self, trunk):
@@ -185,7 +191,8 @@ class OVNTrunkDriver(trunk_base.DriverBase):
     @property
     def is_loaded(self):
         try:
-            return OVN_ML2_MECH_DRIVER_NAME in cfg.CONF.ml2.mechanism_drivers
+            return (ovn_const.OVN_ML2_MECH_DRIVER_NAME in
+                    cfg.CONF.ml2.mechanism_drivers)
         except cfg.NoSuchOptError:
             return False
 
@@ -205,7 +212,7 @@ def register(self, resource, event, trigger, payload=None):
     @classmethod
     def create(cls, plugin_driver):
         cls.plugin_driver = plugin_driver
-        return cls(OVN_ML2_MECH_DRIVER_NAME,
+        return cls(ovn_const.OVN_ML2_MECH_DRIVER_NAME,
                    SUPPORTED_INTERFACES,
                    SUPPORTED_SEGMENTATION_TYPES,
                    None,

neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py

@@ -832,12 +832,12 @@ def _modify_resources_in_nb_db(self):
                 txn.add(self.nb_api.pg_del(pg))
 
             for lport_name in self.reset_lport_dhcpv4_options:
-                txn.add(self.nb_api.set_lswitch_port(lport_name, True,
-                                                     dhcpv4_options=[]))
+                txn.add(self.nb_api.set_lswitch_port(
+                    lport_name, if_exists=True, dhcpv4_options=[]))
 
             for lport_name in self.reset_lport_dhcpv6_options:
-                txn.add(self.nb_api.set_lswitch_port(lport_name, True,
-                                                     dhcpv6_options=[]))
+                txn.add(self.nb_api.set_lswitch_port(
+                    lport_name, if_exists=True, dhcpv6_options=[]))
 
             for dhcp_opts in self.stale_lport_dhcpv4_options:
                 dhcpv4_opts = txn.add(self.nb_api.add_dhcp_options(
@@ -850,7 +850,7 @@ def _modify_resources_in_nb_db(self):
                 if dhcp_opts['port_id'] in self.orphaned_lport_dhcp_options:
                     continue
                 txn.add(self.nb_api.set_lswitch_port(
-                    lport_name, True, dhcpv4_options=dhcpv4_opts))
+                    lport_name, if_exists=True, dhcpv4_options=dhcpv4_opts))
 
             for dhcp_opts in self.stale_lport_dhcpv6_options:
                 dhcpv6_opts = txn.add(self.nb_api.add_dhcp_options(
@@ -863,7 +863,7 @@ def _modify_resources_in_nb_db(self):
                 if dhcp_opts['port_id'] in self.orphaned_lport_dhcp_options:
                     continue
                 txn.add(self.nb_api.set_lswitch_port(
-                    lport_name, True, dhcpv6_options=dhcpv6_opts))
+                    lport_name, if_exists=True, dhcpv6_options=dhcpv6_opts))
 
             for row_uuid in self.missed_dhcp_options:
                 txn.add(self.nb_api.delete_dhcp_options(row_uuid))
@@ -880,12 +880,12 @@ def _modify_resources_in_nb_db(self):
 
             for port_id in self.lport_dhcpv4_disabled:
                 txn.add(self.nb_api.set_lswitch_port(
-                    port_id, True,
+                    port_id, if_exists=True,
                     dhcpv4_options=[self.lport_dhcpv4_disabled[port_id]]))
 
             for port_id in self.lport_dhcpv6_disabled:
                 txn.add(self.nb_api.set_lswitch_port(
-                    port_id, True,
+                    port_id, if_exists=True,
                     dhcpv6_options=[self.lport_dhcpv6_disabled[port_id]]))
 
             # Delete the first DNS record and clear the second row records

neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py

@@ -22,6 +22,8 @@
 from neutron_lib.services.trunk import constants as trunk_consts
 from oslo_utils import uuidutils
 
+from neutron.common.ovn import constants as ovn_const
+
 
 class TestOVNTrunkDriver(base.TestOVNFunctionalBase):
 
@@ -60,18 +62,25 @@ def _get_ovn_trunk_info(self):
         for row in self.nb_api.tables[
                 'Logical_Switch_Port'].rows.values():
             if row.parent_name and row.tag:
+                device_owner = row.external_ids[
+                    ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY]
                 ovn_trunk_info.append({'port_id': row.name,
                                        'parent_port_id': row.parent_name,
-                                       'tag': row.tag})
+                                       'tag': row.tag,
+                                       'device_owner': device_owner,
+                                       })
         return ovn_trunk_info
 
     def _verify_trunk_info(self, trunk, has_items):
         ovn_subports_info = self._get_ovn_trunk_info()
         neutron_subports_info = []
         for subport in trunk.get('sub_ports', []):
-            neutron_subports_info.append({'port_id': subport['port_id'],
-                                          'parent_port_id': [trunk['port_id']],
-                                          'tag': [subport['segmentation_id']]})
+            neutron_subports_info.append(
+                {'port_id': subport['port_id'],
+                 'parent_port_id': [trunk['port_id']],
+                 'tag': [subport['segmentation_id']],
+                 'device_owner': trunk_consts.TRUNK_SUBPORT_OWNER,
+                 })
             # Check that the subport has the binding is active.
             binding = obj_reg.load_class('PortBinding').get_object(
                 self.context, port_id=subport['port_id'], host='')

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py

@@ -200,7 +200,8 @@ def _test_lswitch_port_update_no_exist(self, if_exists=True):
         with mock.patch.object(idlutils, 'row_by_value',
                                side_effect=idlutils.RowNotFound):
             cmd = commands.SetLSwitchPortCommand(
-                self.ovn_api, 'fake-lsp', if_exists=if_exists)
+                self.ovn_api, 'fake-lsp', external_ids_update=None,
+                if_exists=if_exists)
             if if_exists:
                 cmd.run_idl(self.transaction)
             else:
@@ -220,8 +221,8 @@ def test_lswitch_port_update(self):
         with mock.patch.object(idlutils, 'row_by_value',
                                return_value=fake_lsp):
             cmd = commands.SetLSwitchPortCommand(
-                self.ovn_api, fake_lsp.name, if_exists=True,
-                external_ids=new_ext_ids)
+                self.ovn_api, fake_lsp.name, external_ids_update=new_ext_ids,
+                if_exists=True, external_ids=new_ext_ids)
             cmd.run_idl(self.transaction)
             self.assertEqual(new_ext_ids, fake_lsp.external_ids)
 
@@ -255,7 +256,8 @@ def _test_lswitch_port_update_del_dhcp(self, clear_v4_opts,
         with mock.patch.object(idlutils, 'row_by_value',
                                return_value=fake_lsp):
             cmd = commands.SetLSwitchPortCommand(
-                self.ovn_api, fake_lsp.name, if_exists=True, **columns)
+                self.ovn_api, fake_lsp.name, external_ids_update=None,
+                if_exists=True, **columns)
             cmd.run_idl(self.transaction)
 
             if clear_v4_opts and clear_v6_opts:
@@ -307,7 +309,8 @@ def _test_lswitch_port_update_with_dhcp(self, dhcpv4_opts, dhcpv6_opts):
         with mock.patch.object(idlutils, 'row_by_value',
                                return_value=fake_lsp):
             cmd = commands.SetLSwitchPortCommand(
-                self.ovn_api, fake_lsp.name, if_exists=True,
+                self.ovn_api, fake_lsp.name, external_ids_update=ext_ids,
+                if_exists=True,
                 external_ids=ext_ids, dhcpv4_options=dhcpv4_opts,
                 dhcpv6_options=dhcpv6_opts)
             if not isinstance(dhcpv4_opts, list):

neutron/tests/unit/services/trunk/drivers/ovn/test_trunk_driver.py

@@ -129,7 +129,9 @@ def test_create_trunk(self):
 
         calls = [mock.call(lport_name=s_port.port_id,
                            parent_name=trunk.port_id,
-                           tag=s_port.segmentation_id)
+                           tag=s_port.segmentation_id,
+                           external_ids_update={
+                               'neutron:device_owner': 'trunk:subport'})
                  for trunk, s_port in [(self.trunk_1, self.sub_port_1),
                                        (self.trunk_1, self.sub_port_2)]]
         self._assert_calls(self.plugin_driver.nb_ovn.set_lswitch_port, calls)
@@ -196,7 +198,8 @@ def test_delete_trunk(self):
         calls = [mock.call(lport_name=s_port.port_id,
                            parent_name=[],
                            tag=[],
-                           up=False)
+                           up=False,
+                           external_ids_update={'neutron:device_owner': ''})
                  for trunk, s_port in [(self.trunk_1, self.sub_port_1),
                                        (self.trunk_1, self.sub_port_2)]]
         self._assert_calls(self.plugin_driver.nb_ovn.set_lswitch_port, calls)
@@ -225,7 +228,8 @@ def test_delete_trunk_key_not_found(self):
         calls = [mock.call(lport_name=s_port.port_id,
                            parent_name=[],
                            tag=[],
-                           up=False)
+                           up=False,
+                           external_ids_update={'neutron:device_owner': ''})
                  for trunk, s_port in [(self.trunk_1, self.sub_port_1)]]
         self._assert_calls(self.plugin_driver.nb_ovn.set_lswitch_port, calls)