Skip to content

Commit 16ba9b4

Browse files
Zuulopenstack-gerrit
Zuul
authored and
openstack-gerrit
committed
Merge "Retry port_update in the OVN if revision mismatch during live-migration" into stable/yoga
2 parents d7ef3cc + 00b9f52 commit 16ba9b4

File tree

4 files changed

+145
-3
lines changed

4 files changed

+145
-3
lines changed

‎neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
from neutron._i18n import _
4545
from neutron.common.ovn import acl as ovn_acl
4646
from neutron.common.ovn import constants as ovn_const
47+
from neutron.common.ovn import exceptions as ovn_exceptions
4748
from neutron.common.ovn import extensions as ovn_extensions
4849
from neutron.common.ovn import utils as ovn_utils
4950
from neutron.common import utils
@@ -756,6 +757,32 @@ def _validate_ignored_port(self, port, original_port):
756757
'device_owner': original_port['device_owner']})
757758
raise OVNPortUpdateError(resource='port', msg=msg)
758759

760+
def _ovn_update_port(self, plugin_context, port, original_port,
761+
retry_on_revision_mismatch):
762+
try:
763+
self._ovn_client.update_port(plugin_context, port,
764+
port_object=original_port)
765+
except ovn_exceptions.RevisionConflict:
766+
if retry_on_revision_mismatch:
767+
# NOTE(slaweq): I know this is terrible hack but there is no
768+
# other way to workaround possible race between port update
769+
# event from the OVN (port down on the src node) and API
770+
# request from nova-compute to activate binding of the port on
771+
# the dest node.
772+
original_port_migrating_to = original_port.get(
773+
portbindings.PROFILE, {}).get('migrating_to')
774+
port_host_id = port.get(portbindings.HOST_ID)
775+
if (original_port_migrating_to is not None and
776+
original_port_migrating_to == port_host_id):
777+
LOG.debug("Revision number of the port %s has changed "
778+
"probably during live migration. Retrying "
779+
"update port in OVN.", port)
780+
db_port = self._plugin.get_port(plugin_context,
781+
port['id'])
782+
port['revision_number'] = db_port['revision_number']
783+
self._ovn_update_port(plugin_context, port, original_port,
784+
retry_on_revision_mismatch=False)
785+
759786
def create_port_postcommit(self, context):
760787
"""Create a port.
761788
@@ -847,8 +874,8 @@ def update_port_postcommit(self, context):
847874
# will fail that OVN has port with bigger revision.
848875
return
849876

850-
self._ovn_client.update_port(context._plugin_context, port,
851-
port_object=original_port)
877+
self._ovn_update_port(context._plugin_context, port, original_port,
878+
retry_on_revision_mismatch=True)
852879
self._notify_dhcp_updated(port['id'])
853880

854881
def delete_port_postcommit(self, context):

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,11 +269,14 @@ def transaction(self, *args, **kwargs):
269269
This method is just a wrapper around the ovsdbapp transaction
270270
to handle revision conflicts correctly.
271271
"""
272+
revision_mismatch_raise = kwargs.pop('revision_mismatch_raise', False)
272273
try:
273274
with super(OvsdbNbOvnIdl, self).transaction(*args, **kwargs) as t:
274275
yield t
275276
except ovn_exc.RevisionConflict as e:
276277
LOG.info('Transaction aborted. Reason: %s', e)
278+
if revision_mismatch_raise:
279+
raise e
277280

278281
def create_lswitch_port(self, lport_name, lswitch_name, may_exist=True,
279282
**columns):

‎neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,8 @@ def update_port(self, context, port, port_object=None):
594594

595595
check_rev_cmd = self._nb_idl.check_revision_number(
596596
port['id'], port, ovn_const.TYPE_PORTS)
597-
with self._nb_idl.transaction(check_error=True) as txn:
597+
with self._nb_idl.transaction(check_error=True,
598+
revision_mismatch_raise=True) as txn:
598599
txn.add(check_rev_cmd)
599600
columns_dict = {}
600601
if utils.is_lsp_router_port(port):

‎neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545

4646
from neutron.common.ovn import acl as ovn_acl
4747
from neutron.common.ovn import constants as ovn_const
48+
from neutron.common.ovn import exceptions as ovn_exceptions
4849
from neutron.common.ovn import hash_ring_manager
4950
from neutron.common.ovn import utils as ovn_utils
5051
from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
@@ -1960,6 +1961,116 @@ def test_update_port_postcommit_live_migration(
19601961
self.plugin.update_port_status.assert_called_once_with(
19611962
fake_context, fake_port['id'], const.PORT_STATUS_ACTIVE)
19621963

1964+
@mock.patch.object(mech_driver.OVNMechanismDriver,
1965+
'_is_port_provisioning_required', lambda *_: True)
1966+
@mock.patch.object(mech_driver.OVNMechanismDriver, '_notify_dhcp_updated')
1967+
@mock.patch.object(ovn_client.OVNClient, 'update_port')
1968+
def test_update_port_postcommit_live_migration_revision_mismatch_always(
1969+
self, mock_update_port, mock_notify_dhcp):
1970+
self.plugin.update_port_status = mock.Mock()
1971+
self.plugin.get_port = mock.Mock(return_value=mock.MagicMock())
1972+
1973+
fake_context = mock.MagicMock()
1974+
fake_port = fakes.FakePort.create_one_port(
1975+
attrs={
1976+
'status': const.PORT_STATUS_ACTIVE,
1977+
portbindings.PROFILE: {},
1978+
portbindings.VIF_TYPE: portbindings.VIF_TYPE_OVS}).info()
1979+
original_fake_port = fakes.FakePort.create_one_port(
1980+
attrs={
1981+
'status': const.PORT_STATUS_ACTIVE,
1982+
portbindings.PROFILE: {
1983+
ovn_const.MIGRATING_ATTR: fake_port[portbindings.HOST_ID]},
1984+
portbindings.VIF_TYPE: portbindings.VIF_TYPE_OVS}).info()
1985+
1986+
fake_ctx = mock.Mock(current=fake_port, original=original_fake_port,
1987+
_plugin_context=fake_context)
1988+
mock_update_port.side_effect = ovn_exceptions.RevisionConflict(
1989+
resource_id=fake_port['id'],
1990+
resource_type=ovn_const.TYPE_PORTS)
1991+
1992+
self.mech_driver.update_port_postcommit(fake_ctx)
1993+
1994+
self.plugin.update_port_status.assert_not_called()
1995+
self.plugin.get_port.assert_called_once_with(
1996+
mock.ANY, fake_port['id'])
1997+
self.assertEqual(2, mock_update_port.call_count)
1998+
mock_notify_dhcp.assert_called_with(fake_port['id'])
1999+
2000+
@mock.patch.object(mech_driver.OVNMechanismDriver,
2001+
'_is_port_provisioning_required', lambda *_: True)
2002+
@mock.patch.object(mech_driver.OVNMechanismDriver, '_notify_dhcp_updated')
2003+
@mock.patch.object(ovn_client.OVNClient, 'update_port')
2004+
def test_update_port_postcommit_live_migration_revision_mismatch_once(
2005+
self, mock_update_port, mock_notify_dhcp):
2006+
self.plugin.update_port_status = mock.Mock()
2007+
self.plugin.get_port = mock.Mock(return_value=mock.MagicMock())
2008+
2009+
fake_context = mock.MagicMock()
2010+
fake_port = fakes.FakePort.create_one_port(
2011+
attrs={
2012+
'status': const.PORT_STATUS_ACTIVE,
2013+
portbindings.PROFILE: {},
2014+
portbindings.VIF_TYPE: portbindings.VIF_TYPE_OVS}).info()
2015+
original_fake_port = fakes.FakePort.create_one_port(
2016+
attrs={
2017+
'status': const.PORT_STATUS_ACTIVE,
2018+
portbindings.PROFILE: {
2019+
ovn_const.MIGRATING_ATTR: fake_port[portbindings.HOST_ID]},
2020+
portbindings.VIF_TYPE: portbindings.VIF_TYPE_OVS}).info()
2021+
2022+
fake_ctx = mock.Mock(current=fake_port, original=original_fake_port,
2023+
_plugin_context=fake_context)
2024+
mock_update_port.side_effect = [
2025+
ovn_exceptions.RevisionConflict(
2026+
resource_id=fake_port['id'],
2027+
resource_type=ovn_const.TYPE_PORTS),
2028+
None]
2029+
2030+
self.mech_driver.update_port_postcommit(fake_ctx)
2031+
2032+
self.plugin.update_port_status.assert_not_called()
2033+
self.plugin.get_port.assert_called_once_with(
2034+
mock.ANY, fake_port['id'])
2035+
self.assertEqual(2, mock_update_port.call_count)
2036+
mock_notify_dhcp.assert_called_with(fake_port['id'])
2037+
2038+
@mock.patch.object(mech_driver.OVNMechanismDriver,
2039+
'_is_port_provisioning_required', lambda *_: True)
2040+
@mock.patch.object(mech_driver.OVNMechanismDriver, '_notify_dhcp_updated')
2041+
@mock.patch.object(ovn_client.OVNClient, 'update_port')
2042+
def test_update_port_postcommit_revision_mismatch_not_after_live_migration(
2043+
self, mock_update_port, mock_notify_dhcp):
2044+
self.plugin.update_port_status = mock.Mock()
2045+
self.plugin.get_port = mock.Mock(return_value=mock.MagicMock())
2046+
2047+
fake_context = mock.MagicMock()
2048+
fake_port = fakes.FakePort.create_one_port(
2049+
attrs={
2050+
'status': const.PORT_STATUS_ACTIVE,
2051+
portbindings.PROFILE: {},
2052+
portbindings.VIF_TYPE: portbindings.VIF_TYPE_OVS}).info()
2053+
original_fake_port = fakes.FakePort.create_one_port(
2054+
attrs={
2055+
'status': const.PORT_STATUS_DOWN,
2056+
portbindings.PROFILE: {},
2057+
portbindings.VIF_TYPE: portbindings.VIF_TYPE_OVS}).info()
2058+
2059+
fake_ctx = mock.Mock(current=fake_port, original=original_fake_port,
2060+
_plugin_context=fake_context)
2061+
mock_update_port.side_effect = [
2062+
ovn_exceptions.RevisionConflict(
2063+
resource_id=fake_port['id'],
2064+
resource_type=ovn_const.TYPE_PORTS),
2065+
None]
2066+
2067+
self.mech_driver.update_port_postcommit(fake_ctx)
2068+
2069+
self.plugin.update_port_status.assert_not_called()
2070+
self.plugin.get_port.assert_not_called()
2071+
self.assertEqual(1, mock_update_port.call_count)
2072+
mock_notify_dhcp.assert_called_with(fake_port['id'])
2073+
19632074
def _add_chassis(self, nb_cfg):
19642075
chassis_private = mock.Mock()
19652076
chassis_private.nb_cfg = nb_cfg

0 commit comments

Comments
 (0)