Revert "[OVN][Trunk] Add port binding info on subport when parent is bound"
This reverts commit955e621167
. Reason for revert: the port binding handling done in this patch is incorrect and leads to issues during the cold migration process with trunk ports in ML2/OVN. Change-Id: Ifc2d37e2042fad43dd838821953defd99a5f8665 Closes-Bug: #2033887 (cherry picked from commit1b034f8d62
)
This commit is contained in:
parent
a4efc2dd80
commit
cf096344b0
|
@ -51,13 +51,11 @@ class OVNTrunkHandler(object):
|
||||||
context = n_context.get_admin_context()
|
context = n_context.get_admin_context()
|
||||||
db_parent_port = port_obj.Port.get_object(context, id=parent_port)
|
db_parent_port = port_obj.Port.get_object(context, id=parent_port)
|
||||||
parent_port_status = db_parent_port.status
|
parent_port_status = db_parent_port.status
|
||||||
parent_port_bindings = db_parent_port.bindings[0]
|
|
||||||
for subport in subports:
|
for subport in subports:
|
||||||
with db_api.CONTEXT_WRITER.using(context), (
|
with db_api.CONTEXT_WRITER.using(context), (
|
||||||
txn(check_error=True)) as ovn_txn:
|
txn(check_error=True)) as ovn_txn:
|
||||||
port = self._set_binding_profile(context, subport, parent_port,
|
port = self._set_binding_profile(context, subport, parent_port,
|
||||||
parent_port_status,
|
parent_port_status, ovn_txn)
|
||||||
parent_port_bindings, ovn_txn)
|
|
||||||
db_rev.bump_revision(context, port, ovn_const.TYPE_PORTS)
|
db_rev.bump_revision(context, port, ovn_const.TYPE_PORTS)
|
||||||
|
|
||||||
def _unset_sub_ports(self, subports):
|
def _unset_sub_ports(self, subports):
|
||||||
|
@ -71,8 +69,7 @@ class OVNTrunkHandler(object):
|
||||||
|
|
||||||
@db_base_plugin_common.convert_result_to_dict
|
@db_base_plugin_common.convert_result_to_dict
|
||||||
def _set_binding_profile(self, context, subport, parent_port,
|
def _set_binding_profile(self, context, subport, parent_port,
|
||||||
parent_port_status,
|
parent_port_status, ovn_txn):
|
||||||
parent_port_bindings, ovn_txn):
|
|
||||||
LOG.debug("Setting parent %s for subport %s",
|
LOG.debug("Setting parent %s for subport %s",
|
||||||
parent_port, subport.port_id)
|
parent_port, subport.port_id)
|
||||||
db_port = port_obj.Port.get_object(context, id=subport.port_id)
|
db_port = port_obj.Port.get_object(context, id=subport.port_id)
|
||||||
|
@ -84,9 +81,6 @@ class OVNTrunkHandler(object):
|
||||||
check_rev_cmd = self.plugin_driver.nb_ovn.check_revision_number(
|
check_rev_cmd = self.plugin_driver.nb_ovn.check_revision_number(
|
||||||
db_port.id, db_port, ovn_const.TYPE_PORTS)
|
db_port.id, db_port, ovn_const.TYPE_PORTS)
|
||||||
ovn_txn.add(check_rev_cmd)
|
ovn_txn.add(check_rev_cmd)
|
||||||
parent_binding_host = ''
|
|
||||||
if parent_port_bindings.host:
|
|
||||||
parent_binding_host = parent_port_bindings.host
|
|
||||||
try:
|
try:
|
||||||
# NOTE(flaviof): We expect binding's host to be set. Otherwise,
|
# NOTE(flaviof): We expect binding's host to be set. Otherwise,
|
||||||
# sub-port will not transition from DOWN to ACTIVE.
|
# sub-port will not transition from DOWN to ACTIVE.
|
||||||
|
@ -102,7 +96,6 @@ class OVNTrunkHandler(object):
|
||||||
port_obj.PortBinding.update_object(
|
port_obj.PortBinding.update_object(
|
||||||
context,
|
context,
|
||||||
{'profile': binding.profile,
|
{'profile': binding.profile,
|
||||||
'host': parent_binding_host,
|
|
||||||
'vif_type': portbindings.VIF_TYPE_OVS},
|
'vif_type': portbindings.VIF_TYPE_OVS},
|
||||||
port_id=subport.port_id,
|
port_id=subport.port_id,
|
||||||
host=binding.host)
|
host=binding.host)
|
||||||
|
@ -168,14 +161,6 @@ class OVNTrunkHandler(object):
|
||||||
def _is_port_bound(port):
|
def _is_port_bound(port):
|
||||||
return n_utils.is_port_bound(port, log_message=False)
|
return n_utils.is_port_bound(port, log_message=False)
|
||||||
|
|
||||||
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):
|
def trunk_created(self, trunk):
|
||||||
# Check if parent port is handled by OVN.
|
# Check if parent port is handled by OVN.
|
||||||
if not self.plugin_driver.nb_ovn.lookup('Logical_Switch_Port',
|
if not self.plugin_driver.nb_ovn.lookup('Logical_Switch_Port',
|
||||||
|
@ -210,8 +195,6 @@ class OVNTrunkHandler(object):
|
||||||
def trunk_event(self, resource, event, trunk_plugin, payload):
|
def trunk_event(self, resource, event, trunk_plugin, payload):
|
||||||
if event == events.AFTER_CREATE:
|
if event == events.AFTER_CREATE:
|
||||||
self.trunk_created(payload.states[0])
|
self.trunk_created(payload.states[0])
|
||||||
elif event == events.AFTER_UPDATE:
|
|
||||||
self.trunk_updated(payload.states[0])
|
|
||||||
elif event == events.AFTER_DELETE:
|
elif event == events.AFTER_DELETE:
|
||||||
self.trunk_deleted(payload.states[0])
|
self.trunk_deleted(payload.states[0])
|
||||||
elif event == events.PRECOMMIT_CREATE:
|
elif event == events.PRECOMMIT_CREATE:
|
||||||
|
@ -248,9 +231,8 @@ class OVNTrunkDriver(trunk_base.DriverBase):
|
||||||
super(OVNTrunkDriver, self).register(
|
super(OVNTrunkDriver, self).register(
|
||||||
resource, event, trigger, payload=payload)
|
resource, event, trigger, payload=payload)
|
||||||
self._handler = OVNTrunkHandler(self.plugin_driver)
|
self._handler = OVNTrunkHandler(self.plugin_driver)
|
||||||
for _event in (events.AFTER_CREATE, events.AFTER_UPDATE,
|
for _event in (events.AFTER_CREATE, events.AFTER_DELETE,
|
||||||
events.AFTER_DELETE, events.PRECOMMIT_CREATE,
|
events.PRECOMMIT_CREATE, events.PRECOMMIT_DELETE):
|
||||||
events.PRECOMMIT_DELETE):
|
|
||||||
registry.subscribe(self._handler.trunk_event,
|
registry.subscribe(self._handler.trunk_event,
|
||||||
resources.TRUNK,
|
resources.TRUNK,
|
||||||
_event)
|
_event)
|
||||||
|
|
|
@ -17,7 +17,7 @@ import contextlib
|
||||||
from neutron_lib.api.definitions import portbindings
|
from neutron_lib.api.definitions import portbindings
|
||||||
from neutron_lib.callbacks import exceptions as n_exc
|
from neutron_lib.callbacks import exceptions as n_exc
|
||||||
from neutron_lib import constants as n_consts
|
from neutron_lib import constants as n_consts
|
||||||
from neutron_lib.db import api as db_api
|
from neutron_lib.objects import registry as obj_reg
|
||||||
from neutron_lib.plugins import utils
|
from neutron_lib.plugins import utils
|
||||||
from neutron_lib.services.trunk import constants as trunk_consts
|
from neutron_lib.services.trunk import constants as trunk_consts
|
||||||
from oslo_utils import uuidutils
|
from oslo_utils import uuidutils
|
||||||
|
@ -30,8 +30,8 @@ from neutron.tests.functional import base
|
||||||
|
|
||||||
class TestOVNTrunkDriver(base.TestOVNFunctionalBase):
|
class TestOVNTrunkDriver(base.TestOVNFunctionalBase):
|
||||||
|
|
||||||
def setUp(self, **kwargs):
|
def setUp(self):
|
||||||
super().setUp(**kwargs)
|
super(TestOVNTrunkDriver, self).setUp()
|
||||||
self.trunk_plugin = trunk_plugin.TrunkPlugin()
|
self.trunk_plugin = trunk_plugin.TrunkPlugin()
|
||||||
self.trunk_plugin.add_segmentation_type(
|
self.trunk_plugin.add_segmentation_type(
|
||||||
trunk_consts.SEGMENTATION_TYPE_VLAN,
|
trunk_consts.SEGMENTATION_TYPE_VLAN,
|
||||||
|
@ -42,8 +42,7 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase):
|
||||||
sub_ports = sub_ports or []
|
sub_ports = sub_ports or []
|
||||||
with self.network() as network:
|
with self.network() as network:
|
||||||
with self.subnet(network=network) as subnet:
|
with self.subnet(network=network) as subnet:
|
||||||
with self.port(subnet=subnet,
|
with self.port(subnet=subnet) as parent_port:
|
||||||
device_owner='compute:nova') as parent_port:
|
|
||||||
tenant_id = uuidutils.generate_uuid()
|
tenant_id = uuidutils.generate_uuid()
|
||||||
trunk = {'trunk': {
|
trunk = {'trunk': {
|
||||||
'port_id': parent_port['port']['id'],
|
'port_id': parent_port['port']['id'],
|
||||||
|
@ -68,14 +67,17 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase):
|
||||||
if row.parent_name and row.tag:
|
if row.parent_name and row.tag:
|
||||||
device_owner = row.external_ids[
|
device_owner = row.external_ids[
|
||||||
ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY]
|
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,
|
ovn_trunk_info.append({'port_id': row.name,
|
||||||
'parent_port_id': row.parent_name,
|
'parent_port_id': row.parent_name,
|
||||||
'tag': row.tag,
|
'tag': row.tag,
|
||||||
'device_owner': device_owner,
|
'device_owner': device_owner,
|
||||||
|
'revision_number': revision_number,
|
||||||
})
|
})
|
||||||
return ovn_trunk_info
|
return ovn_trunk_info
|
||||||
|
|
||||||
def _verify_trunk_info(self, trunk, has_items, host=''):
|
def _verify_trunk_info(self, trunk, has_items):
|
||||||
ovn_subports_info = self._get_ovn_trunk_info()
|
ovn_subports_info = self._get_ovn_trunk_info()
|
||||||
neutron_subports_info = []
|
neutron_subports_info = []
|
||||||
for subport in trunk.get('sub_ports', []):
|
for subport in trunk.get('sub_ports', []):
|
||||||
|
@ -84,12 +86,12 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase):
|
||||||
'parent_port_id': [trunk['port_id']],
|
'parent_port_id': [trunk['port_id']],
|
||||||
'tag': [subport['segmentation_id']],
|
'tag': [subport['segmentation_id']],
|
||||||
'device_owner': trunk_consts.TRUNK_SUBPORT_OWNER,
|
'device_owner': trunk_consts.TRUNK_SUBPORT_OWNER,
|
||||||
|
'revision_number': '2',
|
||||||
})
|
})
|
||||||
# Check the subport binding.
|
# Check that the subport has the binding is active.
|
||||||
pb = port_obj.PortBinding.get_object(
|
binding = obj_reg.load_class('PortBinding').get_object(
|
||||||
self.context, port_id=subport['port_id'], host=host)
|
self.context, port_id=subport['port_id'], host='')
|
||||||
self.assertEqual(n_consts.PORT_STATUS_ACTIVE, pb.status)
|
self.assertEqual(n_consts.PORT_STATUS_ACTIVE, binding['status'])
|
||||||
self.assertEqual(host, pb.host)
|
|
||||||
|
|
||||||
self.assertCountEqual(ovn_subports_info, neutron_subports_info)
|
self.assertCountEqual(ovn_subports_info, neutron_subports_info)
|
||||||
self.assertEqual(has_items, len(neutron_subports_info) != 0)
|
self.assertEqual(has_items, len(neutron_subports_info) != 0)
|
||||||
|
@ -97,14 +99,6 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase):
|
||||||
if trunk.get('status'):
|
if trunk.get('status'):
|
||||||
self.assertEqual(trunk_consts.TRUNK_ACTIVE_STATUS, trunk['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):
|
def test_trunk_create(self):
|
||||||
with self.trunk() as trunk:
|
with self.trunk() as trunk:
|
||||||
self._verify_trunk_info(trunk, has_items=False)
|
self._verify_trunk_info(trunk, has_items=False)
|
||||||
|
@ -141,22 +135,10 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase):
|
||||||
new_trunk = self.trunk_plugin.get_trunk(self.context,
|
new_trunk = self.trunk_plugin.get_trunk(self.context,
|
||||||
trunk['id'])
|
trunk['id'])
|
||||||
self._verify_trunk_info(new_trunk, has_items=True)
|
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):
|
def test_subport_delete(self):
|
||||||
with self.subport() as subport:
|
with self.subport() as subport:
|
||||||
with self.trunk([subport]) as trunk:
|
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'],
|
self.trunk_plugin.remove_subports(self.context, trunk['id'],
|
||||||
{'sub_ports': [subport]})
|
{'sub_ports': [subport]})
|
||||||
new_trunk = self.trunk_plugin.get_trunk(self.context,
|
new_trunk = self.trunk_plugin.get_trunk(self.context,
|
||||||
|
|
|
@ -122,7 +122,6 @@ class TestTrunkHandler(base.BaseTestCase):
|
||||||
mock.call(mock.ANY,
|
mock.call(mock.ANY,
|
||||||
{'profile': {'parent_name': trunk.port_id,
|
{'profile': {'parent_name': trunk.port_id,
|
||||||
'tag': s_port.segmentation_id},
|
'tag': s_port.segmentation_id},
|
||||||
'host': mock.ANY,
|
|
||||||
'vif_type': portbindings.VIF_TYPE_OVS},
|
'vif_type': portbindings.VIF_TYPE_OVS},
|
||||||
host=mock.ANY,
|
host=mock.ANY,
|
||||||
port_id=s_port.port_id)
|
port_id=s_port.port_id)
|
||||||
|
@ -153,7 +152,6 @@ class TestTrunkHandler(base.BaseTestCase):
|
||||||
self.mock_update_pb.assert_called_once_with(
|
self.mock_update_pb.assert_called_once_with(
|
||||||
mock.ANY, {'profile': {'parent_name': self.sub_port_1.trunk_id,
|
mock.ANY, {'profile': {'parent_name': self.sub_port_1.trunk_id,
|
||||||
'tag': self.sub_port_1.segmentation_id},
|
'tag': self.sub_port_1.segmentation_id},
|
||||||
'host': 'foo.com',
|
|
||||||
'vif_type': portbindings.VIF_TYPE_OVS},
|
'vif_type': portbindings.VIF_TYPE_OVS},
|
||||||
host='foo.com', port_id=self.sub_port_1.port_id)
|
host='foo.com', port_id=self.sub_port_1.port_id)
|
||||||
self.mock_port_update.assert_not_called()
|
self.mock_port_update.assert_not_called()
|
||||||
|
|
Loading…
Reference in New Issue