diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index 2dab8d9d9d4..2e61c9cabc8 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -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( diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index aa2563423ff..c4fc0f55a13 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -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':