[OVN][Trunk] Add port binding info on subport when parent is bound

The host ID and VIF details are added on the subport when the trunk is
created, if it's created when it's not attached to any VM this fields
will remain empty and be filled on the parent port when it gets bound
to a host, but there's no callback to add this info on the subport.

Closes-Bug: #2018289
Closes-Bug: #2024160
Change-Id: I34bb6f178c314907bdf9f76789777f6736938b67
(cherry picked from commit 955e621)

Author: averdagu

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

@@ -49,11 +49,13 @@ def _set_sub_ports(self, parent_port, subports):
         context = n_context.get_admin_context()
         db_parent_port = port_obj.Port.get_object(context, id=parent_port)
         parent_port_status = db_parent_port.status
+        parent_port_bindings = db_parent_port.bindings[0]
         for subport in subports:
             with db_api.CONTEXT_WRITER.using(context), (
                     txn(check_error=True)) as ovn_txn:
                 port = self._set_binding_profile(context, subport, parent_port,
-                                                 parent_port_status, ovn_txn)
+                                                 parent_port_status,
+                                                 parent_port_bindings, ovn_txn)
             db_rev.bump_revision(context, port, ovn_const.TYPE_PORTS)
 
     def _unset_sub_ports(self, subports):
@@ -67,7 +69,8 @@ def _unset_sub_ports(self, subports):
 
     @db_base_plugin_common.convert_result_to_dict
     def _set_binding_profile(self, context, subport, parent_port,
-                             parent_port_status, ovn_txn):
+                             parent_port_status,
+                             parent_port_bindings, ovn_txn):
         LOG.debug("Setting parent %s for subport %s",
                   parent_port, subport.port_id)
         db_port = port_obj.Port.get_object(context, id=subport.port_id)
@@ -79,6 +82,9 @@ def _set_binding_profile(self, context, subport, parent_port,
         check_rev_cmd = self.plugin_driver.nb_ovn.check_revision_number(
             db_port.id, db_port, ovn_const.TYPE_PORTS)
         ovn_txn.add(check_rev_cmd)
+        parent_binding_host = ''
+        if parent_port_bindings.host:
+            parent_binding_host = parent_port_bindings.host
         try:
             # NOTE(flaviof): We expect binding's host to be set. Otherwise,
             # sub-port will not transition from DOWN to ACTIVE.
@@ -94,6 +100,7 @@ def _set_binding_profile(self, context, subport, parent_port,
                 port_obj.PortBinding.update_object(
                     context,
                     {'profile': binding.profile,
+                     'host': parent_binding_host,
                      'vif_type': portbindings.VIF_TYPE_OVS},
                     port_id=subport.port_id,
                     host=binding.host)
@@ -155,6 +162,14 @@ def _unset_binding_profile(self, context, subport, ovn_txn):
         LOG.debug("Done unsetting parent for subport %s", subport.port_id)
         return db_port
 
+    def trunk_updated(self, trunk):
+        # Check if parent port is handled by OVN.
+        if not self.plugin_driver.nb_ovn.lookup('Logical_Switch_Port',
+                                                trunk.port_id, default=None):
+            return
+        if trunk.sub_ports:
+            self._set_sub_ports(trunk.port_id, trunk.sub_ports)
+
     def trunk_created(self, trunk):
         # Check if parent port is handled by OVN.
         if not self.plugin_driver.nb_ovn.lookup('Logical_Switch_Port',
@@ -189,6 +204,8 @@ def subports_deleted(self, trunk, subports):
     def trunk_event(self, resource, event, trunk_plugin, payload):
         if event == events.AFTER_CREATE:
             self.trunk_created(payload.states[0])
+        elif event == events.AFTER_UPDATE:
+            self.trunk_updated(payload.states[0])
         elif event == events.AFTER_DELETE:
             self.trunk_deleted(payload.states[0])
 
@@ -215,13 +232,16 @@ def register(self, resource, event, trigger, payload=None):
         super(OVNTrunkDriver, self).register(
             resource, event, trigger, payload=payload)
         self._handler = OVNTrunkHandler(self.plugin_driver)
-        for trunk_event in (events.AFTER_CREATE, events.AFTER_DELETE):
+        for _event in (events.AFTER_CREATE, events.AFTER_UPDATE,
+                       events.AFTER_DELETE):
             registry.subscribe(self._handler.trunk_event,
                                resources.TRUNK,
-                               trunk_event)
+                               _event)
+
+        for _event in (events.AFTER_CREATE, events.AFTER_DELETE):
             registry.subscribe(self._handler.subport_event,
                                resources.SUBPORTS,
-                               trunk_event)
+                               _event)
 
     @classmethod
     def create(cls, plugin_driver):

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

@@ -14,21 +14,23 @@
 
 import contextlib
 
-from neutron.services.trunk import plugin as trunk_plugin
-from neutron.tests.functional import base
+from neutron_lib.api.definitions import portbindings
 from neutron_lib import constants as n_consts
-from neutron_lib.objects import registry as obj_reg
+from neutron_lib.db import api as db_api
 from neutron_lib.plugins import utils
 from neutron_lib.services.trunk import constants as trunk_consts
 from oslo_utils import uuidutils
 
 from neutron.common.ovn import constants as ovn_const
+from neutron.objects import ports as port_obj
+from neutron.services.trunk import plugin as trunk_plugin
+from neutron.tests.functional import base
 
 
 class TestOVNTrunkDriver(base.TestOVNFunctionalBase):
 
-    def setUp(self):
-        super(TestOVNTrunkDriver, self).setUp()
+    def setUp(self, **kwargs):
+        super().setUp(**kwargs)
         self.trunk_plugin = trunk_plugin.TrunkPlugin()
         self.trunk_plugin.add_segmentation_type(
             trunk_consts.SEGMENTATION_TYPE_VLAN,
@@ -39,7 +41,8 @@ def trunk(self, sub_ports=None):
         sub_ports = sub_ports or []
         with self.network() as network:
             with self.subnet(network=network) as subnet:
-                with self.port(subnet=subnet) as parent_port:
+                with self.port(subnet=subnet,
+                               device_owner='compute:nova') as parent_port:
                     tenant_id = uuidutils.generate_uuid()
                     trunk = {'trunk': {
                         'port_id': parent_port['port']['id'],
@@ -64,17 +67,14 @@ def _get_ovn_trunk_info(self):
             if row.parent_name and row.tag:
                 device_owner = row.external_ids[
                     ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY]
-                revision_number = row.external_ids[
-                    ovn_const.OVN_REV_NUM_EXT_ID_KEY]
                 ovn_trunk_info.append({'port_id': row.name,
                                        'parent_port_id': row.parent_name,
                                        'tag': row.tag,
                                        'device_owner': device_owner,
-                                       'revision_number': revision_number,
                                        })
         return ovn_trunk_info
 
-    def _verify_trunk_info(self, trunk, has_items):
+    def _verify_trunk_info(self, trunk, has_items, host=''):
         ovn_subports_info = self._get_ovn_trunk_info()
         neutron_subports_info = []
         for subport in trunk.get('sub_ports', []):
@@ -83,19 +83,27 @@ def _verify_trunk_info(self, trunk, has_items):
                  'parent_port_id': [trunk['port_id']],
                  'tag': [subport['segmentation_id']],
                  'device_owner': trunk_consts.TRUNK_SUBPORT_OWNER,
-                 'revision_number': '2',
                  })
-            # 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='')
-            self.assertEqual(n_consts.PORT_STATUS_ACTIVE, binding['status'])
+            # Check the subport binding.
+            pb = port_obj.PortBinding.get_object(
+                self.context, port_id=subport['port_id'], host=host)
+            self.assertEqual(n_consts.PORT_STATUS_ACTIVE, pb.status)
+            self.assertEqual(host, pb.host)
 
         self.assertCountEqual(ovn_subports_info, neutron_subports_info)
         self.assertEqual(has_items, len(neutron_subports_info) != 0)
 
         if trunk.get('status'):
             self.assertEqual(trunk_consts.TRUNK_ACTIVE_STATUS, trunk['status'])
 
+    def _bind_port(self, port_id, host):
+        with db_api.CONTEXT_WRITER.using(self.context):
+            pb = port_obj.PortBinding.get_object(self.context,
+                                                 port_id=port_id, host='')
+            pb.delete()
+            port_obj.PortBinding(self.context, port_id=port_id, host=host,
+                                 vif_type=portbindings.VIF_TYPE_OVS).create()
+
     def test_trunk_create(self):
         with self.trunk() as trunk:
             self._verify_trunk_info(trunk, has_items=False)
@@ -113,10 +121,22 @@ def test_subport_add(self):
                 new_trunk = self.trunk_plugin.get_trunk(self.context,
                                                         trunk['id'])
                 self._verify_trunk_info(new_trunk, has_items=True)
+                # Bind parent port. That will trigger the binding of the
+                # trunk subports too, using the same host ID.
+                self._bind_port(trunk['port_id'], 'host1')
+                self.mech_driver.set_port_status_up(trunk['port_id'])
+                self._verify_trunk_info(new_trunk, has_items=True,
+                                        host='host1')
 
     def test_subport_delete(self):
         with self.subport() as subport:
             with self.trunk([subport]) as trunk:
+                # Bind parent port.
+                self._bind_port(trunk['port_id'], 'host1')
+                self.mech_driver.set_port_status_up(trunk['port_id'])
+                self._verify_trunk_info(trunk, has_items=True,
+                                        host='host1')
+
                 self.trunk_plugin.remove_subports(self.context, trunk['id'],
                                                   {'sub_ports': [subport]})
                 new_trunk = self.trunk_plugin.get_trunk(self.context,

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

@@ -122,6 +122,7 @@ def test_create_trunk(self):
             mock.call(mock.ANY,
                       {'profile': {'parent_name': trunk.port_id,
                                    'tag': s_port.segmentation_id},
+                       'host': mock.ANY,
                        'vif_type': portbindings.VIF_TYPE_OVS},
                       host=mock.ANY,
                       port_id=s_port.port_id)
@@ -152,6 +153,7 @@ def test_create_trunk_port_db_exception(self):
         self.mock_update_pb.assert_called_once_with(
             mock.ANY, {'profile': {'parent_name': self.sub_port_1.trunk_id,
                                    'tag': self.sub_port_1.segmentation_id},
+                       'host': 'foo.com',
                        'vif_type': portbindings.VIF_TYPE_OVS},
             host='foo.com', port_id=self.sub_port_1.port_id)
         self.mock_port_update.assert_not_called()