Rollback router intf port update if csnat update fails
If a CSNAT port update fails when a subnet is added, it will leave the regular updated router port in the changed state since it's a separate port update with a separate transaction. So when the CSNAT update fails we need explicitly undo the 1st update with another update. Closes-Bug: #1609693 Change-Id: If979aebbebf6da77f3fcec0cc7ec8b27e388aba4
This commit is contained in:
parent
f2c481ee3c
commit
bd076a0983
|
@ -22,7 +22,7 @@ from oslo_log import log as logging
|
|||
from oslo_utils import excutils
|
||||
import six
|
||||
|
||||
from neutron._i18n import _, _LI, _LW
|
||||
from neutron._i18n import _, _LE, _LI, _LW
|
||||
from neutron.callbacks import events
|
||||
from neutron.callbacks import exceptions
|
||||
from neutron.callbacks import registry
|
||||
|
@ -30,6 +30,7 @@ from neutron.callbacks import resources
|
|||
from neutron.common import constants as l3_const
|
||||
from neutron.common import utils as n_utils
|
||||
from neutron.db.allowed_address_pairs import models as addr_pair_db
|
||||
from neutron.db import api as db_api
|
||||
from neutron.db import l3_agentschedulers_db as l3_sched_db
|
||||
from neutron.db import l3_attrs_db
|
||||
from neutron.db import l3_db
|
||||
|
@ -352,9 +353,41 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
|
|||
if cs_port:
|
||||
fixed_ips = list(cs_port['port']['fixed_ips'])
|
||||
fixed_ips.append(fixed_ip)
|
||||
updated_port = self._core_plugin.update_port(
|
||||
context.elevated(),
|
||||
cs_port['port_id'], {'port': {'fixed_ips': fixed_ips}})
|
||||
try:
|
||||
updated_port = self._core_plugin.update_port(
|
||||
context.elevated(),
|
||||
cs_port['port_id'],
|
||||
{'port': {'fixed_ips': fixed_ips}})
|
||||
except Exception:
|
||||
with excutils.save_and_reraise_exception():
|
||||
# we need to try to undo the updated router
|
||||
# interface from above so it's not out of sync
|
||||
# with the csnat port.
|
||||
# TODO(kevinbenton): switch to taskflow to manage
|
||||
# these rollbacks.
|
||||
@db_api.retry_db_errors
|
||||
def revert():
|
||||
# TODO(kevinbenton): even though we get the
|
||||
# port each time, there is a potential race
|
||||
# where we update the port with stale IPs if
|
||||
# another interface operation is occuring at
|
||||
# the same time. This can be fixed in the
|
||||
# future with a compare-and-swap style update
|
||||
# using the revision number of the port.
|
||||
p = self._core_plugin.get_port(
|
||||
context.elevated(), port['id'])
|
||||
upd = {'port': {'fixed_ips': [
|
||||
ip for ip in p['fixed_ips']
|
||||
if ip['subnet_id'] != fixed_ip['subnet_id']
|
||||
]}}
|
||||
self._core_plugin.update_port(
|
||||
context.elevated(), port['id'], upd)
|
||||
try:
|
||||
revert()
|
||||
except Exception:
|
||||
LOG.exception(_LE("Failed to revert change "
|
||||
"to router port %s."),
|
||||
port['id'])
|
||||
LOG.debug("CSNAT port updated for IPv6 subnet: "
|
||||
"%s", updated_port)
|
||||
router_interface_info = self._make_router_interface_info(
|
||||
|
|
|
@ -17,6 +17,7 @@ import mock
|
|||
from neutron_lib import constants as l3_const
|
||||
from neutron_lib import exceptions
|
||||
from oslo_utils import uuidutils
|
||||
import testtools
|
||||
|
||||
from neutron.common import constants as n_const
|
||||
from neutron import context
|
||||
|
@ -555,6 +556,45 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
|
|||
self.mixin.manager = manager
|
||||
return router, subnet_v4, subnet_v6
|
||||
|
||||
def test_undo_router_interface_change_on_csnat_error(self):
|
||||
self._test_undo_router_interface_change_on_csnat_error(False)
|
||||
|
||||
def test_undo_router_interface_change_on_csnat_error_revert_failure(self):
|
||||
self._test_undo_router_interface_change_on_csnat_error(True)
|
||||
|
||||
def _test_undo_router_interface_change_on_csnat_error(self, fail_revert):
|
||||
router, subnet_v4, subnet_v6 = self._setup_router_with_v4_and_v6()
|
||||
net = {'network': {'id': subnet_v6['subnet']['network_id'],
|
||||
'tenant_id': subnet_v6['subnet']['tenant_id']}}
|
||||
orig_update = self.mixin._core_plugin.update_port
|
||||
|
||||
def update_port(*args, **kwargs):
|
||||
# 1st port update is the interface, 2nd is csnat, 3rd is revert
|
||||
# we want to simulate errors after the 1st
|
||||
update_port.calls += 1
|
||||
if update_port.calls == 2:
|
||||
raise RuntimeError('csnat update failure')
|
||||
if update_port.calls == 3 and fail_revert:
|
||||
# this is to ensure that if the revert fails, the original
|
||||
# exception is raised (not this ValueError)
|
||||
raise ValueError('failure from revert')
|
||||
return orig_update(*args, **kwargs)
|
||||
update_port.calls = 0
|
||||
self.mixin._core_plugin.update_port = update_port
|
||||
|
||||
with self.subnet(network=net, cidr='fe81::/64',
|
||||
gateway_ip='fe81::1', ip_version=6) as subnet2_v6:
|
||||
with testtools.ExpectedException(RuntimeError):
|
||||
self.mixin.add_router_interface(self.ctx, router['id'],
|
||||
{'subnet_id': subnet2_v6['subnet']['id']})
|
||||
if fail_revert:
|
||||
# a revert failure will mean the interface is still added
|
||||
# so we can't re-add it
|
||||
return
|
||||
# starting over should work if first interface was cleaned up
|
||||
self.mixin.add_router_interface(self.ctx, router['id'],
|
||||
{'subnet_id': subnet2_v6['subnet']['id']})
|
||||
|
||||
def test_remove_router_interface_csnat_ports_removal_with_ipv6(self):
|
||||
router, subnet_v4, subnet_v6 = self._setup_router_with_v4_and_v6()
|
||||
csnat_filters = {'device_owner':
|
||||
|
|
Loading…
Reference in New Issue