Merge "[OVN][Trunk] Add port binding info on subport when parent is bound" into stable/yoga

Author: Zuul

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