Delete gw port on exceptions
Clean up related core plugin ports on routers when there is a failure creating the record for the router port. The two partial bugs will be fixed after I476d3e03c8ee763cc4be6d679fe9f501eb3a19b5 has merged. Closes-Bug: #1600344 Partial-Bug: #1535225 Partial-Bug: #1535226 Change-Id: I8dd832f35e20d1ee090ebab921f0deea533b6fc8
This commit is contained in:
parent
d78450e1fe
commit
fbd3578d64
|
@ -368,17 +368,18 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
|
||||||
if not gw_port['fixed_ips']:
|
if not gw_port['fixed_ips']:
|
||||||
LOG.debug('No IPs available for external network %s',
|
LOG.debug('No IPs available for external network %s',
|
||||||
network_id)
|
network_id)
|
||||||
|
with p_utils.delete_port_on_error(self._core_plugin,
|
||||||
with context.session.begin(subtransactions=True):
|
context.elevated(), gw_port['id']):
|
||||||
router.gw_port = self._core_plugin._get_port(context.elevated(),
|
with context.session.begin(subtransactions=True):
|
||||||
gw_port['id'])
|
router.gw_port = self._core_plugin._get_port(
|
||||||
router_port = RouterPort(
|
context.elevated(), gw_port['id'])
|
||||||
router_id=router.id,
|
router_port = RouterPort(
|
||||||
port_id=gw_port['id'],
|
router_id=router.id,
|
||||||
port_type=DEVICE_OWNER_ROUTER_GW
|
port_id=gw_port['id'],
|
||||||
)
|
port_type=DEVICE_OWNER_ROUTER_GW
|
||||||
context.session.add(router)
|
)
|
||||||
context.session.add(router_port)
|
context.session.add(router)
|
||||||
|
context.session.add(router_port)
|
||||||
|
|
||||||
def _validate_gw_info(self, context, gw_port, info, ext_ips):
|
def _validate_gw_info(self, context, gw_port, info, ext_ips):
|
||||||
network_id = info['network_id'] if info else None
|
network_id = info['network_id'] if info else None
|
||||||
|
@ -746,6 +747,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
|
||||||
|
|
||||||
# This should be True unless adding an IPv6 prefix to an existing port
|
# This should be True unless adding an IPv6 prefix to an existing port
|
||||||
new_port = True
|
new_port = True
|
||||||
|
cleanup_port = False
|
||||||
|
|
||||||
if add_by_port:
|
if add_by_port:
|
||||||
port, subnets = self._add_interface_by_port(
|
port, subnets = self._add_interface_by_port(
|
||||||
|
@ -755,15 +757,19 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
|
||||||
else:
|
else:
|
||||||
port, subnets, new_port = self._add_interface_by_subnet(
|
port, subnets, new_port = self._add_interface_by_subnet(
|
||||||
context, router, interface_info['subnet_id'], device_owner)
|
context, router, interface_info['subnet_id'], device_owner)
|
||||||
|
cleanup_port = new_port # only cleanup port we created
|
||||||
|
|
||||||
if new_port:
|
if new_port:
|
||||||
with context.session.begin(subtransactions=True):
|
with p_utils.delete_port_on_error(self._core_plugin,
|
||||||
router_port = RouterPort(
|
context, port['id']) as delmgr:
|
||||||
port_id=port['id'],
|
delmgr.delete_on_error = cleanup_port
|
||||||
router_id=router.id,
|
with context.session.begin(subtransactions=True):
|
||||||
port_type=device_owner
|
router_port = RouterPort(
|
||||||
)
|
port_id=port['id'],
|
||||||
context.session.add(router_port)
|
router_id=router.id,
|
||||||
|
port_type=device_owner
|
||||||
|
)
|
||||||
|
context.session.add(router_port)
|
||||||
|
|
||||||
gw_ips = []
|
gw_ips = []
|
||||||
gw_network_id = None
|
gw_network_id = None
|
||||||
|
|
|
@ -307,6 +307,7 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
|
||||||
|
|
||||||
# This should be True unless adding an IPv6 prefix to an existing port
|
# This should be True unless adding an IPv6 prefix to an existing port
|
||||||
new_port = True
|
new_port = True
|
||||||
|
cleanup_port = False
|
||||||
|
|
||||||
if add_by_port:
|
if add_by_port:
|
||||||
port, subnets = self._add_interface_by_port(
|
port, subnets = self._add_interface_by_port(
|
||||||
|
@ -314,31 +315,27 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
|
||||||
elif add_by_sub:
|
elif add_by_sub:
|
||||||
port, subnets, new_port = self._add_interface_by_subnet(
|
port, subnets, new_port = self._add_interface_by_subnet(
|
||||||
context, router, interface_info['subnet_id'], device_owner)
|
context, router, interface_info['subnet_id'], device_owner)
|
||||||
|
cleanup_port = new_port
|
||||||
|
|
||||||
subnet = subnets[0]
|
subnet = subnets[0]
|
||||||
|
|
||||||
if new_port:
|
if new_port:
|
||||||
if router.extra_attributes.distributed and router.gw_port:
|
with p_utils.delete_port_on_error(self._core_plugin,
|
||||||
try:
|
context, port['id']) as delmgr:
|
||||||
|
if router.extra_attributes.distributed and router.gw_port:
|
||||||
admin_context = context.elevated()
|
admin_context = context.elevated()
|
||||||
self._add_csnat_router_interface_port(
|
self._add_csnat_router_interface_port(
|
||||||
admin_context, router, port['network_id'],
|
admin_context, router, port['network_id'],
|
||||||
port['fixed_ips'][-1]['subnet_id'])
|
port['fixed_ips'][-1]['subnet_id'])
|
||||||
except Exception:
|
|
||||||
with excutils.save_and_reraise_exception():
|
|
||||||
# we need to preserve the original state prior
|
|
||||||
# the request by rolling back the port creation
|
|
||||||
# that led to new_port=True
|
|
||||||
self._core_plugin.delete_port(
|
|
||||||
admin_context, port['id'], l3_port_check=False)
|
|
||||||
|
|
||||||
with context.session.begin(subtransactions=True):
|
delmgr.delete_on_error = cleanup_port
|
||||||
router_port = l3_db.RouterPort(
|
with context.session.begin(subtransactions=True):
|
||||||
port_id=port['id'],
|
router_port = l3_db.RouterPort(
|
||||||
router_id=router.id,
|
port_id=port['id'],
|
||||||
port_type=device_owner
|
router_id=router.id,
|
||||||
)
|
port_type=device_owner
|
||||||
context.session.add(router_port)
|
)
|
||||||
|
context.session.add(router_port)
|
||||||
|
|
||||||
# NOTE: For IPv6 additional subnets added to the same
|
# NOTE: For IPv6 additional subnets added to the same
|
||||||
# network we need to update the CSNAT port with respective
|
# network we need to update the CSNAT port with respective
|
||||||
|
|
|
@ -16,6 +16,7 @@
|
||||||
Common utilities and helper functions for OpenStack Networking Plugins.
|
Common utilities and helper functions for OpenStack Networking Plugins.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
import contextlib
|
||||||
import hashlib
|
import hashlib
|
||||||
|
|
||||||
from neutron_lib import constants as n_const
|
from neutron_lib import constants as n_const
|
||||||
|
@ -23,9 +24,10 @@ from neutron_lib import exceptions
|
||||||
from oslo_config import cfg
|
from oslo_config import cfg
|
||||||
from oslo_log import log as logging
|
from oslo_log import log as logging
|
||||||
from oslo_utils import encodeutils
|
from oslo_utils import encodeutils
|
||||||
|
from oslo_utils import excutils
|
||||||
import webob.exc
|
import webob.exc
|
||||||
|
|
||||||
from neutron._i18n import _, _LI
|
from neutron._i18n import _, _LE, _LI
|
||||||
from neutron.api.v2 import attributes
|
from neutron.api.v2 import attributes
|
||||||
from neutron.callbacks import events
|
from neutron.callbacks import events
|
||||||
from neutron.callbacks import registry
|
from neutron.callbacks import registry
|
||||||
|
@ -195,6 +197,26 @@ def create_port(core_plugin, context, port, check_allow_post=True):
|
||||||
return core_plugin.create_port(context, {'port': port_data})
|
return core_plugin.create_port(context, {'port': port_data})
|
||||||
|
|
||||||
|
|
||||||
|
class _DelManager(object):
|
||||||
|
def __init__(self):
|
||||||
|
self.delete_on_error = True
|
||||||
|
|
||||||
|
|
||||||
|
@contextlib.contextmanager
|
||||||
|
def delete_port_on_error(core_plugin, context, port_id):
|
||||||
|
mgr = _DelManager()
|
||||||
|
try:
|
||||||
|
yield mgr
|
||||||
|
except Exception:
|
||||||
|
with excutils.save_and_reraise_exception():
|
||||||
|
try:
|
||||||
|
if mgr.delete_on_error:
|
||||||
|
core_plugin.delete_port(context, port_id,
|
||||||
|
l3_port_check=False)
|
||||||
|
except Exception:
|
||||||
|
LOG.exception(_LE("Failed to cleanup port: %s"), port_id)
|
||||||
|
|
||||||
|
|
||||||
def get_interface_name(name, prefix='', max_len=n_const.DEVICE_NAME_MAX_LEN):
|
def get_interface_name(name, prefix='', max_len=n_const.DEVICE_NAME_MAX_LEN):
|
||||||
"""Construct an interface name based on the prefix and name.
|
"""Construct an interface name based on the prefix and name.
|
||||||
|
|
||||||
|
|
|
@ -23,6 +23,7 @@ from oslo_utils import uuidutils
|
||||||
import testscenarios
|
import testscenarios
|
||||||
from webob import exc
|
from webob import exc
|
||||||
|
|
||||||
|
from neutron import context as nctx
|
||||||
from neutron.db import api as db_api
|
from neutron.db import api as db_api
|
||||||
from neutron.db import external_net_db
|
from neutron.db import external_net_db
|
||||||
from neutron.db import l3_db
|
from neutron.db import l3_db
|
||||||
|
@ -30,6 +31,7 @@ from neutron.db import l3_gwmode_db
|
||||||
from neutron.db import models_v2
|
from neutron.db import models_v2
|
||||||
from neutron.extensions import l3
|
from neutron.extensions import l3
|
||||||
from neutron.extensions import l3_ext_gw_mode
|
from neutron.extensions import l3_ext_gw_mode
|
||||||
|
from neutron import manager
|
||||||
from neutron.tests import base
|
from neutron.tests import base
|
||||||
from neutron.tests.unit.db import test_db_base_plugin_v2
|
from neutron.tests.unit.db import test_db_base_plugin_v2
|
||||||
from neutron.tests.unit.extensions import test_l3
|
from neutron.tests.unit.extensions import test_l3
|
||||||
|
@ -390,6 +392,20 @@ class ExtGwModeIntTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase,
|
||||||
expected_code=expected_code,
|
expected_code=expected_code,
|
||||||
neutron_context=neutron_context)
|
neutron_context=neutron_context)
|
||||||
|
|
||||||
|
def test_router_gateway_set_fail_after_port_create(self):
|
||||||
|
with self.router() as r, self.subnet() as s:
|
||||||
|
ext_net_id = s['subnet']['network_id']
|
||||||
|
self._set_net_external(ext_net_id)
|
||||||
|
plugin = manager.NeutronManager.get_plugin()
|
||||||
|
with mock.patch.object(plugin, '_get_port',
|
||||||
|
side_effect=ValueError()):
|
||||||
|
self._set_router_external_gateway(r['router']['id'],
|
||||||
|
ext_net_id,
|
||||||
|
expected_code=500)
|
||||||
|
ports = [p for p in plugin.get_ports(nctx.get_admin_context())
|
||||||
|
if p['device_owner'] == l3_db.DEVICE_OWNER_ROUTER_GW]
|
||||||
|
self.assertFalse(ports)
|
||||||
|
|
||||||
def test_router_gateway_set_retry(self):
|
def test_router_gateway_set_retry(self):
|
||||||
with self.router() as r, self.subnet() as s:
|
with self.router() as r, self.subnet() as s:
|
||||||
ext_net_id = s['subnet']['network_id']
|
ext_net_id = s['subnet']['network_id']
|
||||||
|
|
|
@ -16,6 +16,7 @@ import hashlib
|
||||||
|
|
||||||
import mock
|
import mock
|
||||||
from neutron_lib import constants
|
from neutron_lib import constants
|
||||||
|
import testtools
|
||||||
|
|
||||||
from neutron.plugins.common import utils
|
from neutron.plugins.common import utils
|
||||||
from neutron.tests import base
|
from neutron.tests import base
|
||||||
|
@ -70,3 +71,30 @@ class TestUtils(base.BaseTestCase):
|
||||||
self.assertEqual(12, len(utils.get_interface_name(LONG_NAME1,
|
self.assertEqual(12, len(utils.get_interface_name(LONG_NAME1,
|
||||||
prefix="pre-",
|
prefix="pre-",
|
||||||
max_len=12)))
|
max_len=12)))
|
||||||
|
|
||||||
|
def test_delete_port_on_error(self):
|
||||||
|
core_plugin, context = mock.Mock(), mock.Mock()
|
||||||
|
port_id = 'pid'
|
||||||
|
with testtools.ExpectedException(ValueError):
|
||||||
|
with utils.delete_port_on_error(core_plugin, context, port_id):
|
||||||
|
raise ValueError()
|
||||||
|
core_plugin.delete_port.assert_called_once_with(context, port_id,
|
||||||
|
l3_port_check=False)
|
||||||
|
|
||||||
|
def test_delete_port_on_error_no_delete(self):
|
||||||
|
core_plugin, context = mock.Mock(), mock.Mock()
|
||||||
|
with testtools.ExpectedException(ValueError):
|
||||||
|
with utils.delete_port_on_error(core_plugin, context, 1) as cm:
|
||||||
|
cm.delete_on_error = False
|
||||||
|
raise ValueError()
|
||||||
|
self.assertFalse(core_plugin.delete_port.called)
|
||||||
|
|
||||||
|
def test_delete_port_on_error_fail_port_delete(self):
|
||||||
|
core_plugin, context = mock.Mock(), mock.Mock()
|
||||||
|
core_plugin.delete_port.side_effect = TypeError()
|
||||||
|
port_id = 'pid'
|
||||||
|
with testtools.ExpectedException(ValueError):
|
||||||
|
with utils.delete_port_on_error(core_plugin, context, port_id):
|
||||||
|
raise ValueError()
|
||||||
|
core_plugin.delete_port.assert_called_once_with(context, port_id,
|
||||||
|
l3_port_check=False)
|
||||||
|
|
Loading…
Reference in New Issue