Skip to content

Commit

Permalink
Separate NVP create lport operation and neutron db transaction
Browse files Browse the repository at this point in the history 
Bug 1200001

This patch removes the NVP call for creating a logical port from
the SQL transaction context used for creating the Neutron port.
It also ensures orphaned data are properly removed from both
the Neutron DB and the NVP backend.

Change-Id: I028a1493ecf732f2422e0eaa2020bac4ebdbb457
  • Loading branch information
@salv-orlando
salv-orlando committed Jul 30, 2013
1 parent c0d3c58 commit 270fd80
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 54 deletions.
149 changes: 95 additions & 54 deletions neutron/plugins/nicira/NeutronPlugin.py
View file
Expand Up @@ -435,6 +435,22 @@ def _nvp_create_port_helper(self, cluster, ls_uuid, port_data,
port_data[ext_qos.QUEUE],
port_data.get(mac_ext.MAC_LEARNING))

def _handle_create_port_exception(self, context, port_id,
ls_uuid, lp_uuid):
with excutils.save_and_reraise_exception():
# rollback nvp logical port only if it was successfully
# created on NVP. Should this command fail the original
# exception will be raised.
if lp_uuid:
# Remove orphaned port from NVP
nvplib.delete_port(self.cluster, ls_uuid, lp_uuid)
# rollback the neutron-nvp port mapping
nicira_db.delete_neutron_nvp_port_mapping(context.session,
port_id)
msg = (_("An exception occured while creating the "
"quantum port %s on the NVP plaform") % port_id)
LOG.exception(msg)

def _nvp_create_port(self, context, port_data):
"""Driver for creating a logical switch port on NVP platform."""
# FIXME(salvatore-orlando): On the NVP platform we do not really have
Expand All @@ -448,6 +464,8 @@ def _nvp_create_port(self, context, port_data):
port_data['network_id'])
# No need to actually update the DB state - the default is down
return port_data
lport = None
selected_lswitch = None
try:
selected_lswitch = self._nvp_find_lswitch_for_port(context,
port_data)
Expand All @@ -466,11 +484,11 @@ def _nvp_create_port(self, context, port_data):
LOG.debug(_("_nvp_create_port completed for port %(name)s "
"on network %(network_id)s. The new port id is "
"%(id)s."), port_data)
except NvpApiClient.NvpApiException:
msg = (_("An exception occured while plugging the interface "
"into network:%s") % port_data['network_id'])
LOG.exception(msg)
raise q_exc.NeutronException(message=msg)
except (NvpApiClient.NvpApiException, q_exc.NeutronException):
self._handle_create_port_exception(
context, port_data['id'],
selected_lswitch and selected_lswitch['uuid'],
lport and lport['uuid'])

def _nvp_delete_port(self, context, port_data):
# FIXME(salvatore-orlando): On the NVP platform we do not really have
Expand Down Expand Up @@ -515,7 +533,7 @@ def _nvp_delete_router_port(self, context, port_data):
lrouter_id,
port_data['network_id'],
nvp_port_id)
except (NvpApiClient.NvpApiException, NvpApiClient.ResourceNotFound):
except NvpApiClient.NvpApiException:
# Do not raise because the issue might as well be that the
# router has already been deleted, so there would be nothing
# to do here
Expand All @@ -534,18 +552,26 @@ def _nvp_create_router_port(self, context, port_data):
err_msg=(_("It is not allowed to create router interface "
"ports on external networks as '%s'") %
port_data['network_id']))
selected_lswitch = self._nvp_find_lswitch_for_port(context,
port_data)
# Do not apply port security here!
lport = self._nvp_create_port_helper(self.cluster,
selected_lswitch['uuid'],
port_data,
False)
nicira_db.add_neutron_nvp_port_mapping(
context.session, port_data['id'], lport['uuid'])
LOG.debug(_("_nvp_create_port completed for port %(name)s on "
"network %(network_id)s. The new port id is %(id)s."),
port_data)
lport = None
selected_lswitch = None
try:
selected_lswitch = self._nvp_find_lswitch_for_port(
context, port_data)
# Do not apply port security here!
lport = self._nvp_create_port_helper(
self.cluster, selected_lswitch['uuid'],
port_data, False)
nicira_db.add_neutron_nvp_port_mapping(
context.session, port_data['id'], lport['uuid'])
LOG.debug(_("_nvp_create_router_port completed for port "
"%(name)s on network %(network_id)s. The new "
"port id is %(id)s."),
port_data)
except (NvpApiClient.NvpApiException, q_exc.NeutronException):
self._handle_create_port_exception(
context, port_data['id'],
selected_lswitch and selected_lswitch['uuid'],
lport and lport['uuid'])

def _find_router_gw_port(self, context, port_data):
router_id = port_data['device_id']
Expand Down Expand Up @@ -651,21 +677,30 @@ def _nvp_create_l2_gw_port(self, context, port_data):
port_data['network_id'])
# No need to actually update the DB state - the default is down
return port_data
selected_lswitch = self._nvp_find_lswitch_for_port(context,
port_data)
lport = self._nvp_create_port_helper(self.cluster,
selected_lswitch['uuid'],
port_data,
True)
nicira_db.add_neutron_nvp_port_mapping(
context.session, port_data['id'], lport['uuid'])
nvplib.plug_l2_gw_service(
self.cluster,
port_data['network_id'],
lport['uuid'],
port_data['device_id'],
int(port_data.get('gw:segmentation_id') or 0))
LOG.debug(_("_nvp_create_port completed for port %(name)s "
lport = None
try:
selected_lswitch = self._nvp_find_lswitch_for_port(
context, port_data)
lport = self._nvp_create_port_helper(
self.cluster,
selected_lswitch['uuid'],
port_data,
True)
nicira_db.add_neutron_nvp_port_mapping(
context.session, port_data['id'], lport['uuid'])
nvplib.plug_l2_gw_service(
self.cluster,
port_data['network_id'],
lport['uuid'],
port_data['device_id'],
int(port_data.get('gw:segmentation_id') or 0))
except Exception:
with excutils.save_and_reraise_exception():
if lport:
nvplib.delete_port(self.cluster,
selected_lswitch['uuid'],
lport['uuid'])
LOG.debug(_("_nvp_create_l2_gw_port completed for port %(name)s "
"on network %(network_id)s. The new port id "
"is %(id)s."), port_data)

Expand Down Expand Up @@ -1205,6 +1240,7 @@ def create_port(self, context, port):
with context.session.begin(subtransactions=True):
# First we allocate port in neutron database
neutron_db = super(NvpPluginV2, self).create_port(context, port)
neutron_port_id = neutron_db['id']
# Update fields obtained from neutron db (eg: MAC address)
port["port"].update(neutron_db)
# metadata_dhcp_host_route
Expand Down Expand Up @@ -1236,27 +1272,6 @@ def create_port(self, context, port):
self._create_mac_learning_state(context, port_data)
elif mac_ext.MAC_LEARNING in port_data:
port_data.pop(mac_ext.MAC_LEARNING)
# provider networking extension checks
# Fetch the network and network binding from neutron db
try:
port_data = port['port'].copy()
port_create_func = self._port_drivers['create'].get(
port_data['device_owner'],
self._port_drivers['create']['default'])

port_create_func(context, port_data)
except q_exc.NotFound:
LOG.warning(_("Network %s was not found in NVP."),
port_data['network_id'])
port_data['status'] = constants.PORT_STATUS_ERROR
except Exception:
with excutils.save_and_reraise_exception():
# FIXME (arosen) or the plugin_interface call failed in
# which case we need to garbage collect the left over
# port in nvp.
err_msg = _("Unable to create port or set port "
"attachment in NVP.")
LOG.error(err_msg)

LOG.debug(_("create_port completed on NVP for tenant "
"%(tenant_id)s: (%(id)s)"), port_data)
Expand All @@ -1267,6 +1282,32 @@ def create_port(self, context, port):
self._extend_port_qos_queue(context, port_data)
self._process_portbindings_create_and_update(context,
port, port_data)
# DB Operation is complete, perform NVP operation
try:
port_data = port['port'].copy()
port_create_func = self._port_drivers['create'].get(
port_data['device_owner'],
self._port_drivers['create']['default'])
port_create_func(context, port_data)
except q_exc.NotFound:
LOG.warning(_("Logical switch for network %s was not "
"found in NVP."), port_data['network_id'])
# Put port in error on quantum DB
with context.session.begin(subtransactions=True):
port = self._get_port(context, neutron_port_id)
port_data['status'] = constants.PORT_STATUS_ERROR
port['status'] = port_data['status']
context.session.add(port)
except Exception:
# Port must be removed from Quantum DB
with excutils.save_and_reraise_exception():
LOG.error(_("Unable to create port or set port "
"attachment in NVP."))
with context.session.begin(subtransactions=True):
self._delete_port(context, neutron_port_id)

# Port has been created both on DB and NVP - proceed with
# scheduling network and notifying DHCP agent
net = self.get_network(context, port_data['network_id'])
self.schedule_network(context, net)
if notify_dhcp_agent:
Expand Down
5 changes: 5 additions & 0 deletions neutron/plugins/nicira/dbexts/nicira_db.py
View file
Expand Up @@ -72,6 +72,11 @@ def get_nvp_port_id(session, neutron_id):
return


def delete_neutron_nvp_port_mapping(session, neutron_id):
return (session.query(nicira_models.NeutronNvpPortMapping).
filter_by(quantum_id=neutron_id).delete())


def unset_default_network_gateways(session):
with session.begin(subtransactions=True):
session.query(nicira_networkgw_db.NetworkGateway).update(
Expand Down
30 changes: 30 additions & 0 deletions neutron/tests/unit/nicira/test_nicira_plugin.py
View file
Expand Up @@ -21,6 +21,7 @@
import webob.exc

from neutron.common import constants
from neutron.common import exceptions as ntn_exc
import neutron.common.test_lib as test_lib
from neutron import context
from neutron.extensions import l3
Expand All @@ -29,6 +30,7 @@
from neutron.extensions import securitygroup as secgrp
from neutron import manager
from neutron.openstack.common import uuidutils
from neutron.plugins.nicira.dbexts import nicira_db
from neutron.plugins.nicira.dbexts import nicira_qos_db as qos_db
from neutron.plugins.nicira.extensions import nvp_networkgw
from neutron.plugins.nicira.extensions import nvp_qos as ext_qos
Expand Down Expand Up @@ -162,6 +164,34 @@ def test_create_port_name_exceeds_40_chars(self):
# Assert the neutron name is not truncated
self.assertEqual(name, port['port']['name'])

def _verify_no_orphan_left(self, net_id):
# Verify no port exists on net
# ie: cleanup on db was successful
query_params = "network_id=%s" % net_id
self._test_list_resources('port', [],
query_params=query_params)
# Also verify no orphan port was left on nvp
# no port should be there at all
self.assertFalse(self.fc._fake_lswitch_lport_dict)

def test_create_port_nvp_error_no_orphan_left(self):
with mock.patch.object(nvplib, 'create_lport',
side_effect=NvpApiClient.NvpApiException):
with self.network() as net:
net_id = net['network']['id']
self._create_port(self.fmt, net_id,
webob.exc.HTTPInternalServerError.code)
self._verify_no_orphan_left(net_id)

def test_create_port_neutron_error_no_orphan_left(self):
with mock.patch.object(nicira_db, 'add_neutron_nvp_port_mapping',
side_effect=ntn_exc.NeutronException):
with self.network() as net:
net_id = net['network']['id']
self._create_port(self.fmt, net_id,
webob.exc.HTTPInternalServerError.code)
self._verify_no_orphan_left(net_id)


class TestNiciraNetworksV2(test_plugin.TestNetworksV2,
NiciraPluginV2TestCase):
Expand Down

0 comments on commit 270fd80

Please sign in to comment.