Browse Source

Revert "Set binding profile directly from OVNTrunkDriver"

During recent days we many observed failures of 
neutron_tempest_plugin.scenario.test_trunk.TrunkTest.test_trunk_subport_lifecycle test. 
It started to fail on a different asserts. 
Lets revert this patch, as it is a regression, and work further on this issue.

This reverts commit 41f6d622ab.

Related-Bug: #1834637
Related-Bug: #1845479

Change-Id: I63acf0c809aa158aeba1c79c7c9083966ffd58a3
changes/06/685206/7
Maciej Józefczyk 2 years ago
parent
commit
2e0832f7b8
  1. 102
      networking_ovn/ml2/trunk_driver.py
  2. 284
      networking_ovn/tests/unit/ml2/test_trunk_driver.py

102
networking_ovn/ml2/trunk_driver.py

@ -10,7 +10,6 @@
# License for the specific language governing permissions and limitations
# under the License.
from neutron.objects import ports as port_obj
from neutron.services.trunk.drivers import base as trunk_base
from neutron_lib.api.definitions import portbindings
from neutron_lib.callbacks import events
@ -20,6 +19,7 @@ from neutron_lib import context as n_context
from neutron_lib import exceptions as n_exc
from neutron_lib.services.trunk import constants as trunk_consts
from oslo_config import cfg
from oslo_db import exception as os_db_exc
from oslo_log import log
from networking_ovn.common.constants import OVN_ML2_MECH_DRIVER_NAME
@ -40,97 +40,43 @@ LOG = log.getLogger(__name__)
class OVNTrunkHandler(object):
def __init__(self, plugin_driver):
self.plugin_driver = plugin_driver
self.context = n_context.get_admin_context()
def _set_binding_profile(self, port_id, parent_port, tag=None):
context = n_context.get_admin_context()
binding_profile = {}
if parent_port and tag:
binding_profile = {'parent_name': parent_port, 'tag': tag}
port = {'port': {'binding:profile': binding_profile}}
if not tag:
port['port']['binding:host_id'] = None
try:
self.plugin_driver._plugin.update_port(context, port_id, port)
except (os_db_exc.DBReferenceError, n_exc.PortNotFound):
LOG.debug("Port not found trying to set binding_profile: %s",
port_id)
def _set_sub_ports(self, parent_port, subports):
txn = self.plugin_driver._nb_ovn.transaction
with self.context.session.begin(subtransactions=True), (
txn(check_error=True)) as ovn_txn:
for port in subports:
self._set_binding_profile(port, parent_port, ovn_txn)
for port in subports:
self._set_binding_profile(port.port_id, parent_port,
tag=port.segmentation_id)
def _unset_sub_ports(self, subports):
txn = self.plugin_driver._nb_ovn.transaction
with self.context.session.begin(subtransactions=True), (
txn(check_error=True)) as ovn_txn:
for port in subports:
self._unset_binding_profile(port, ovn_txn)
def _set_binding_profile(self, subport, parent_port, ovn_txn):
db_port = port_obj.Port.get_object(self.context, id=subport.port_id)
if not db_port:
LOG.debug("Port not found while trying to set "
"binding_profile: %s",
subport.port_id)
return
try:
for binding in db_port.bindings:
binding.profile.update({
'parent_name': parent_port,
'tag': subport.segmentation_id})
# host + port_id is primary key
port_obj.PortBinding.update_object(
self.context, {'profile': binding.profile},
port_id=subport.port_id,
host=binding.host)
except n_exc.ObjectNotFound:
LOG.debug("Port not found while trying to set "
"binding_profile: %s", subport.port_id)
return
ovn_txn.add(self.plugin_driver._nb_ovn.set_lswitch_port(
lport_name=subport.port_id,
parent_name=parent_port,
tag=subport.segmentation_id))
def _unset_binding_profile(self, subport, ovn_txn):
db_port = port_obj.Port.get_object(self.context, id=subport.port_id)
if not db_port:
LOG.debug("Port not found while trying to unset "
"binding_profile: %s",
subport.port_id)
return
try:
for binding in db_port.bindings:
binding.profile.pop('tag', None)
binding.profile.pop('parent_name', None)
# host + port_id is primary key
port_obj.PortBinding.update_object(
self.context,
{'profile': binding.profile,
'vif_type': portbindings.VIF_TYPE_UNBOUND,
'vif_details': '',
'host': ''},
port_id=subport.port_id,
host=binding.host)
port_obj.PortBindingLevel.delete_objects(
self.context, port_id=subport.port_id, host=binding.host)
except n_exc.ObjectNotFound:
LOG.debug("Port not found while trying to unset "
"binding_profile: %s", subport.port_id)
return
ovn_txn.add(self.plugin_driver._nb_ovn.set_lswitch_port(
lport_name=subport.port_id,
parent_name=[],
up=False,
tag=[]))
for port in subports:
self._set_binding_profile(port.port_id, None)
def trunk_created(self, trunk):
if trunk.sub_ports:
self._set_sub_ports(trunk.port_id, trunk.sub_ports)
self._set_sub_ports(trunk.port_id, trunk.sub_ports)
trunk.update(status=trunk_consts.TRUNK_ACTIVE_STATUS)
def trunk_deleted(self, trunk):
if trunk.sub_ports:
self._unset_sub_ports(trunk.sub_ports)
self._unset_sub_ports(trunk.sub_ports)
def subports_added(self, trunk, subports):
if subports:
self._set_sub_ports(trunk.port_id, subports)
self._set_sub_ports(trunk.port_id, subports)
trunk.update(status=trunk_consts.TRUNK_ACTIVE_STATUS)
def subports_deleted(self, trunk, subports):
if subports:
self._unset_sub_ports(subports)
self._unset_sub_ports(subports)
trunk.update(status=trunk_consts.TRUNK_ACTIVE_STATUS)
def trunk_event(self, resource, event, trunk_plugin, payload):

284
networking_ovn/tests/unit/ml2/test_trunk_driver.py

@ -15,21 +15,15 @@
import mock
from neutron.tests import base
from neutron_lib.api.definitions import portbindings
from neutron_lib.callbacks import events
from neutron_lib.callbacks import registry
from neutron_lib.callbacks import resources
from neutron_lib import exceptions as n_exc
from neutron_lib.services.trunk import constants as trunk_consts
from oslo_config import cfg
from networking_ovn.common.constants import OVN_ML2_MECH_DRIVER_NAME
from networking_ovn.ml2 import trunk_driver
from networking_ovn.tests.unit import fakes
from neutron.objects.ports import Port
from neutron.objects.ports import PortBinding
class TestTrunkHandler(base.BaseTestCase):
def setUp(self):
@ -41,24 +35,20 @@ class TestTrunkHandler(base.BaseTestCase):
self.plugin_driver._nb_ovn = fakes.FakeOvsdbNbOvnIdl()
self.handler = trunk_driver.OVNTrunkHandler(self.plugin_driver)
self.trunk_1 = mock.Mock()
self.trunk_1.port_id = "trunk-1"
self.trunk_1.port_id = "parent_port_1"
self.trunk_2 = mock.Mock()
self.trunk_2.port_id = "trunk-2"
self.trunk_2.port_id = "parent_port_2"
self.sub_port_1 = mock.Mock()
self.sub_port_1.segmentation_id = 40
self.sub_port_1.trunk_id = "trunk-1"
self.sub_port_1.port_id = "sub_port_1"
self.sub_port_1_obj = self._get_fake_port_obj(
port_id='sub_port_1')
self.sub_port_2 = mock.Mock()
self.sub_port_2.segmentation_id = 41
self.sub_port_2.trunk_id = "trunk-1"
self.sub_port_2.port_id = "sub_port_2"
self.sub_port_2_obj = self._get_fake_port_obj(
port_id='sub_port_1')
self.sub_port_3 = mock.Mock()
self.sub_port_3.segmentation_id = 42
@ -74,187 +64,80 @@ class TestTrunkHandler(base.BaseTestCase):
"neutron.objects.trunk.Trunk.get_object").start()
self.get_trunk_object.side_effect = lambda ctxt, id: \
self.trunk_1 if id == 'trunk-1' else self.trunk_2
self.mock_get_port = mock.patch(
"neutron.objects.ports.Port.get_object").start()
self.mock_get_port.side_effect = lambda ctxt, id: (
self.sub_port_1_obj if id == 'sub_port_1' else (
self.sub_port_2_obj if id == 'sub_port_2' else None))
self.mock_update_pb = mock.patch(
"neutron.objects.ports.PortBinding.update_object").start()
self.mock_clear_levels = mock.patch(
"neutron.objects.ports.PortBindingLevel.delete_objects").start()
def _get_fake_port_obj(self, port_id):
with mock.patch('uuid.UUID') as mock_uuid:
mock_uuid.return_value = port_id
port = Port()
port.id = port_id
port.bindings = [PortBinding(profile={}, host='foo.com')]
return port
def _assert_calls(self, mock, expected_calls):
self.assertEqual(
len(expected_calls),
mock.call_count)
mock.assert_has_calls(
expected_calls, any_order=True)
def _get_binding_profile_info(self, parent_name=None, tag=None):
binding_profile = {}
if parent_name and tag:
binding_profile = {'parent_name': parent_name, 'tag': tag}
info = {'port': {'binding:profile': binding_profile}}
if not tag:
info['port']['binding:host_id'] = None
return info
def _assert_update_port_calls(self, calls):
self.assertEqual(len(calls),
self.plugin_driver._plugin.update_port.call_count)
self.plugin_driver._plugin.update_port.assert_has_calls(
calls, any_order=True)
def test_create_trunk(self):
self.trunk_1.sub_ports = []
self.handler.trunk_created(self.trunk_1)
self.plugin_driver._nb_ovn.set_lswitch_port.assert_not_called()
self.mock_update_pb.assert_not_called()
self.plugin_driver._plugin.update_port.assert_not_called()
self.trunk_1.sub_ports = [self.sub_port_1, self.sub_port_2]
self.handler.trunk_created(self.trunk_1)
calls = [
mock.call(mock.ANY,
{'profile': {'parent_name': trunk.port_id,
'tag': s_port.segmentation_id}},
host=mock.ANY,
port_id=s_port.port_id)
for trunk, s_port in [(self.trunk_1, self.sub_port_1),
(self.trunk_1, self.sub_port_2)]]
self._assert_calls(self.mock_update_pb, calls)
calls = [mock.call(lport_name=s_port.port_id,
parent_name=trunk.port_id,
tag=s_port.segmentation_id)
calls = [mock.call(mock.ANY, s_port.port_id,
self._get_binding_profile_info(
trunk.port_id, s_port.segmentation_id))
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)
self.mock_clear_levels.assert_not_called()
def test_create_trunk_port_not_found(self):
self.trunk_1.sub_ports = [self.sub_port_4]
self.handler.trunk_created(self.trunk_1)
self.plugin_driver._nb_ovn.set_lswitch_port.assert_not_called()
self.mock_update_pb.assert_not_called()
def test_create_trunk_port_db_exception(self):
self.trunk_1.sub_ports = [self.sub_port_1]
self.mock_update_pb.side_effect = [n_exc.ObjectNotFound(id=1)]
self.handler.trunk_created(self.trunk_1)
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', port_id=self.sub_port_1.port_id)
self.plugin_driver._nb_ovn.set_lswitch_port.assert_not_called()
self._assert_update_port_calls(calls)
def test_delete_trunk(self):
self.trunk_1.sub_ports = []
self.handler.trunk_deleted(self.trunk_1)
self.plugin_driver._nb_ovn.set_lswitch_port.assert_not_called()
self.mock_update_pb.assert_not_called()
self.mock_clear_levels.assert_not_called()
self.plugin_driver._plugin.update_port.assert_not_called()
self.trunk_1.sub_ports = [self.sub_port_1, self.sub_port_2]
self.sub_port_1_obj.bindings[0].profile.update({
'tag': self.sub_port_1.segmentation_id,
'parent_name': self.sub_port_1.trunk_id,
'foo_field': self.sub_port_1.trunk_id})
self.sub_port_2_obj.bindings[0].profile.update({
'tag': self.sub_port_2.segmentation_id,
'parent_name': self.sub_port_2.trunk_id,
'foo_field': self.sub_port_2.trunk_id})
self.handler.trunk_deleted(self.trunk_1)
calls = [
mock.call(
mock.ANY,
{'profile': {'foo_field': s_port.trunk_id},
'host': '',
'vif_type': portbindings.VIF_TYPE_UNBOUND,
'vif_details': ''},
host='foo.com',
port_id=s_port.port_id)
for trunk, s_port in [(self.trunk_1, self.sub_port_1),
(self.trunk_1, self.sub_port_2)]]
self._assert_calls(self.mock_update_pb, calls)
calls = [
mock.call(mock.ANY,
host='foo.com',
port_id=s_port.port_id)
for trunk, s_port in [(self.trunk_1, self.sub_port_1),
(self.trunk_1, self.sub_port_2)]]
self._assert_calls(self.mock_clear_levels, calls)
calls = [mock.call(lport_name=s_port.port_id,
parent_name=[],
tag=[],
up=False)
calls = [mock.call(mock.ANY, s_port.port_id,
self._get_binding_profile_info(
trunk.port_id, None))
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)
def test_delete_trunk_key_not_found(self):
self.sub_port_1_obj.bindings[0].profile.update({
'foo_field': self.sub_port_1.trunk_id})
self.trunk_1.sub_ports = [self.sub_port_1]
self.handler.trunk_deleted(self.trunk_1)
calls = [
mock.call(mock.ANY,
{'profile': {'foo_field': s_port.trunk_id},
'host': '',
'vif_type': portbindings.VIF_TYPE_UNBOUND,
'vif_details': ''},
host='foo.com',
port_id=s_port.port_id)
for trunk, s_port in [(self.trunk_1, self.sub_port_1)]]
self._assert_calls(self.mock_update_pb, calls)
calls = [
mock.call(mock.ANY,
host='foo.com',
port_id=s_port.port_id)
for trunk, s_port in [(self.trunk_1, self.sub_port_1)]]
self._assert_calls(self.mock_clear_levels, calls)
calls = [mock.call(lport_name=s_port.port_id,
parent_name=[],
tag=[],
up=False)
for trunk, s_port in [(self.trunk_1, self.sub_port_1)]]
self._assert_calls(self.plugin_driver._nb_ovn.set_lswitch_port, calls)
def test_delete_trunk_port_not_found(self):
self.trunk_1.sub_ports = [self.sub_port_4]
self.handler.trunk_deleted(self.trunk_1)
self.plugin_driver._nb_ovn.set_lswitch_port.assert_not_called()
self.mock_update_pb.assert_not_called()
self.mock_clear_levels.assert_not_called()
def test_delete_trunk_port_db_exception(self):
self.trunk_1.sub_ports = [self.sub_port_1]
self.mock_update_pb.side_effect = [n_exc.ObjectNotFound(id=1)]
self.handler.trunk_deleted(self.trunk_1)
self.mock_update_pb.assert_called_once_with(
mock.ANY, {'profile': {},
'vif_type': portbindings.VIF_TYPE_UNBOUND,
'host': '', 'vif_details': ''},
host='foo.com', port_id=self.sub_port_1.port_id)
self.plugin_driver._nb_ovn.set_lswitch_port.assert_not_called()
self.mock_clear_levels.assert_not_called()
self._assert_update_port_calls(calls)
def test_subports_added(self):
with mock.patch.object(self.handler, '_set_sub_ports') as set_s:
self.handler.subports_added(self.trunk_1,
[self.sub_port_1, self.sub_port_2])
set_s.assert_called_once_with(
self.trunk_1.port_id, [self.sub_port_1, self.sub_port_2])
self.trunk_1.update.assert_called_once_with(
status=trunk_consts.TRUNK_ACTIVE_STATUS)
self.handler.subports_added(self.trunk_1,
[self.sub_port_1, self.sub_port_2])
self.handler.subports_added(self.trunk_2,
[self.sub_port_3, self.sub_port_4])
calls = [mock.call(mock.ANY, s_port.port_id,
self._get_binding_profile_info(
trunk.port_id, s_port.segmentation_id))
for trunk, s_port in [(self.trunk_1, self.sub_port_1),
(self.trunk_1, self.sub_port_2),
(self.trunk_2, self.sub_port_3),
(self.trunk_2, self.sub_port_4)]]
self._assert_update_port_calls(calls)
def test_subports_deleted(self):
with mock.patch.object(self.handler, '_unset_sub_ports') as unset_s:
self.handler.subports_deleted(self.trunk_1,
[self.sub_port_1, self.sub_port_2])
unset_s.assert_called_once_with(
[self.sub_port_1, self.sub_port_2])
self.trunk_1.update.assert_called_once_with(
status=trunk_consts.TRUNK_ACTIVE_STATUS)
self.handler.subports_deleted(self.trunk_1,
[self.sub_port_1, self.sub_port_2])
self.handler.subports_deleted(self.trunk_2,
[self.sub_port_3, self.sub_port_4])
calls = [mock.call(mock.ANY, s_port.port_id,
self._get_binding_profile_info(
trunk.port_id, None))
for trunk, s_port in [(self.trunk_1, self.sub_port_1),
(self.trunk_1, self.sub_port_2),
(self.trunk_2, self.sub_port_3),
(self.trunk_2, self.sub_port_4)]]
self._assert_update_port_calls(calls)
def _fake_trunk_event_payload(self):
payload = mock.Mock()
@ -274,33 +157,33 @@ class TestTrunkHandler(base.BaseTestCase):
payload.original_trunk.sub_ports = [original_subport]
return payload
@mock.patch.object(trunk_driver.OVNTrunkHandler, '_set_sub_ports')
def test_trunk_event_create(self, set_subports):
def test_trunk_event_create(self):
fake_payload = self._fake_trunk_event_payload()
self.handler.trunk_event(
mock.ANY, events.AFTER_CREATE, mock.ANY, fake_payload)
set_subports.assert_called_once_with(
fake_payload.current_trunk.port_id,
fake_payload.current_trunk.sub_ports)
fake_payload.current_trunk.update.assert_called_once_with(
status=trunk_consts.TRUNK_ACTIVE_STATUS)
@mock.patch.object(trunk_driver.OVNTrunkHandler, '_unset_sub_ports')
def test_trunk_event_delete(self, unset_subports):
self.plugin_driver._plugin.update_port.assert_called_once_with(
mock.ANY, fake_payload.current_trunk.sub_ports[0].port_id,
self._get_binding_profile_info(
fake_payload.current_trunk.port_id,
fake_payload.current_trunk.sub_ports[0].segmentation_id))
def test_trunk_event_delete(self):
fake_payload = self._fake_trunk_event_payload()
self.handler.trunk_event(
mock.ANY, events.AFTER_DELETE, mock.ANY, fake_payload)
unset_subports.assert_called_once_with(
fake_payload.original_trunk.sub_ports)
@mock.patch.object(trunk_driver.OVNTrunkHandler, '_set_sub_ports')
@mock.patch.object(trunk_driver.OVNTrunkHandler, '_unset_sub_ports')
def test_trunk_event_invalid(self, unset_subports, set_subports):
self.plugin_driver._plugin.update_port.assert_called_once_with(
mock.ANY, fake_payload.original_trunk.sub_ports[0].port_id,
self._get_binding_profile_info(
fake_payload.original_trunk.port_id, None))
def test_trunk_event_invalid(self):
fake_payload = self._fake_trunk_event_payload()
self.handler.trunk_event(
mock.ANY, events.BEFORE_DELETE, mock.ANY, fake_payload)
set_subports.assert_not_called()
unset_subports.assert_not_called()
self.plugin_driver._plugin.update_port.assert_not_called()
def _fake_subport_event_payload(self):
payload = mock.Mock()
@ -313,30 +196,33 @@ class TestTrunkHandler(base.BaseTestCase):
payload.subports = [original_subport]
return payload
@mock.patch.object(trunk_driver.OVNTrunkHandler, 'subports_added')
def test_subport_event_create(self, s_added):
def test_subport_event_create(self):
fake_payload = self._fake_subport_event_payload()
self.handler.subport_event(
mock.ANY, events.AFTER_CREATE, mock.ANY, fake_payload)
s_added.assert_called_once_with(
fake_payload.original_trunk, fake_payload.subports)
@mock.patch.object(trunk_driver.OVNTrunkHandler, 'subports_deleted')
def test_subport_event_delete(self, s_deleted):
self.plugin_driver._plugin.update_port.assert_called_once_with(
mock.ANY, fake_payload.subports[0].port_id,
self._get_binding_profile_info(
fake_payload.original_trunk.port_id,
fake_payload.subports[0].segmentation_id))
def test_subport_event_delete(self):
fake_payload = self._fake_subport_event_payload()
self.handler.subport_event(
mock.ANY, events.AFTER_DELETE, mock.ANY, fake_payload)
s_deleted.assert_called_once_with(
fake_payload.original_trunk, fake_payload.subports)
@mock.patch.object(trunk_driver.OVNTrunkHandler, 'subports_added')
@mock.patch.object(trunk_driver.OVNTrunkHandler, 'subports_deleted')
def test_subport_event_invalid(self, s_deleted, s_added):
self.plugin_driver._plugin.update_port.assert_called_once_with(
mock.ANY, fake_payload.subports[0].port_id,
self._get_binding_profile_info(
fake_payload.original_trunk.port_id, None))
def test_subport_event_invalid(self):
fake_payload = self._fake_trunk_event_payload()
self.handler.subport_event(
mock.ANY, events.BEFORE_DELETE, mock.ANY, fake_payload)
s_added.assert_not_called()
s_deleted.assert_not_called()
self.plugin_driver._plugin.update_port.assert_not_called()
class TestTrunkDriver(base.BaseTestCase):

Loading…
Cancel
Save