x/vmware-nsx
Code Issues Proposed changes

Merge "nec plugin: allow to delete resource with ERROR status"

Browse Source
This commit is contained in:
Jenkins 2014-03-27 16:54:05 +00:00 committed by Gerrit Code Review
commit 15c16ddd8a
5 changed files with 161 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

84
neutron/plugins/nec/nec_plugin.py
View File

@ -167,6 +167,14 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
obj_db = obj_getter(context, id)
obj_db.update(request)
def _update_resource_status_if_changed(self, context, resource_type,
resource_dict, new_status):
if resource_dict['status'] != new_status:
self._update_resource_status(context, resource_type,
resource_dict['id'],
new_status)
resource_dict['status'] = new_status
def _check_ofc_tenant_in_use(self, context, tenant_id):
"""Check if the specified tenant is used."""
# All networks are created on OFC
@ -234,16 +242,18 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
return port
def deactivate_port(self, context, port):
def deactivate_port(self, context, port, raise_exc=True):
"""Deactivate port by deleting port from OFC if exists."""
if not self.ofc.exists_ofc_port(context, port['id']):
LOG.debug(_("deactivate_port(): skip, ofc_port does not "
"exist."))
LOG.debug(_("deactivate_port(): skip, ofc_port for port=%s "
"does not exist."), port['id'])
return port
try:
self.ofc.delete_ofc_port(context, port['id'], port)
port_status = const.PORT_STATUS_DOWN
self._update_resource_status_if_changed(
context, "port", port, const.PORT_STATUS_DOWN)
return port
except (nexc.OFCResourceNotFound, nexc.OFCMappingNotFound):
# There is a case where multiple delete_port operation are
# running concurrently. For example, delete_port from
@ -261,15 +271,14 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
port['status'] = const.PORT_STATUS_DOWN
return port
except nexc.OFCException as exc:
LOG.error(_("delete_ofc_port() failed due to %s"), exc)
port_status = const.PORT_STATUS_ERROR
if port_status != port['status']:
self._update_resource_status(context, "port", port['id'],
port_status)
port['status'] = port_status
return port
with excutils.save_and_reraise_exception() as ctxt:
LOG.error(_("Failed to delete port=%(port)s from OFC: "
"%(exc)s"), {'port': port['id'], 'exc': exc})
self._update_resource_status_if_changed(
context, "port", port, const.PORT_STATUS_ERROR)
if not raise_exc:
ctxt.reraise = False
return port
def _net_status(self, network):
# NOTE: NEC Plugin accept admin_state_up. When it's False, this plugin
@ -336,7 +345,11 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
ports = super(NECPluginV2, self).get_ports(context,
filters=filters)
for port in ports:
self.deactivate_port(context, port)
# If some error occurs, status of errored port is set to ERROR.
# This is avoids too many rollback.
# TODO(amotoki): Raise an exception after all port operations
# are finished to inform the caller of API of the failure.
self.deactivate_port(context, port, raise_exc=False)
elif changed and new_net['admin_state_up']:
# enable ports of the network
filters = dict(network_id=[id], status=[const.PORT_STATUS_DOWN],
@ -368,28 +381,25 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
raise n_exc.NetworkInUse(net_id=id)
# Make sure auto-delete ports on OFC are deleted.
_error_ports = []
# If an error occurs during port deletion,
# delete_network will be aborted.
for port in ports:
port = self.deactivate_port(context, port)
if port['status'] == const.PORT_STATUS_ERROR:
_error_ports.append(port['id'])
if _error_ports:
reason = (_("Failed to delete port(s)=%s from OFC.") %
','.join(_error_ports))
raise nexc.OFCException(reason=reason)
# delete all packet_filters of the network from the controller
for pf in net_db.packetfilters:
self.delete_packet_filter(context, pf['id'])
try:
self.ofc.delete_ofc_network(context, id, net_db)
except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
with excutils.save_and_reraise_exception():
reason = _("delete_network() failed due to %s") % exc
LOG.error(reason)
self._update_resource_status(context, "network", net_db['id'],
const.NET_STATUS_ERROR)
if self.ofc.exists_ofc_network(context, id):
try:
self.ofc.delete_ofc_network(context, id, net_db)
except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
with excutils.save_and_reraise_exception():
reason = _("delete_network() failed due to %s") % exc
LOG.error(reason)
self._update_resource_status(
context, "network", net_db['id'],
const.NET_STATUS_ERROR)
super(NECPluginV2, self).delete_network(context, id)
@ -630,10 +640,8 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
port = self._make_port_dict(port_db)
handler = self._get_port_handler('delete', port['device_owner'])
# handler() raises an exception if an error occurs during processing.
port = handler(context, port)
if port['status'] == const.PORT_STATUS_ERROR:
reason = _("Failed to delete port=%s from OFC.") % id
raise nexc.OFCException(reason=reason)
# delete all packet_filters of the port from the controller
for pf in port_db.packetfilters:
@ -741,9 +749,10 @@ class NECPluginV2RPCCallbacks(object):
# NOTE: Make sure that packet filters on this port exist while
# the port is active to avoid unexpected packet transfer.
if portinfo:
self.plugin.deactivate_port(rpc_context, port)
self.plugin.deactivate_packet_filters_by_port(rpc_context,
id)
self.plugin.deactivate_port(rpc_context, port,
raise_exc=False)
self.plugin.deactivate_packet_filters_by_port(
rpc_context, id, raise_exc=False)
self.plugin.activate_packet_filters_by_port(rpc_context, id)
self.plugin.activate_port_if_ready(rpc_context, port)
for id in kwargs.get('port_removed', []):
@ -764,8 +773,9 @@ class NECPluginV2RPCCallbacks(object):
ndb.del_portinfo(session, id)
port = self._get_port(rpc_context, id)
if port:
self.plugin.deactivate_port(rpc_context, port)
self.plugin.deactivate_packet_filters_by_port(rpc_context, id)
self.plugin.deactivate_port(rpc_context, port, raise_exc=False)
self.plugin.deactivate_packet_filters_by_port(
rpc_context, id, raise_exc=False)
def _get_port(self, context, port_id):
try:

55
neutron/plugins/nec/packet_filter.py
View File

@ -148,12 +148,9 @@ class PacketFilterMixin(pf_db.PacketFilterDbMixin):
# validate ownership
pf = self.get_packet_filter(context, id)
# deactivate_packet_filter() raises an exception
# if an error occurs during processing.
pf = self.deactivate_packet_filter(context, pf)
if pf['status'] == pf_db.PF_STATUS_ERROR:
msg = _("Failed to delete packet_filter id=%s which remains in "
"error status.") % id
LOG.error(msg)
raise nexc.OFCException(reason=msg)
super(PacketFilterMixin, self).delete_packet_filter(context, id)
@ -205,30 +202,28 @@ class PacketFilterMixin(pf_db.PacketFilterDbMixin):
LOG.debug(_("deactivate_packet_filter_if_ready() called, "
"packet_filter=%s."), packet_filter)
pf_id = packet_filter['id']
current = packet_filter['status']
pf_status = current
if self.ofc.exists_ofc_packet_filter(context, pf_id):
LOG.debug(_("deactivate_packet_filter(): "
"deleting packet_filter id=%s from OFC."), pf_id)
try:
self.ofc.delete_ofc_packet_filter(context, pf_id)
pf_status = pf_db.PF_STATUS_DOWN
except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
LOG.error(_("Failed to delete packet_filter id=%(id)s from "
"OFC: %(exc)s"), {'id': pf_id, 'exc': exc})
pf_status = pf_db.PF_STATUS_ERROR
else:
if not self.ofc.exists_ofc_packet_filter(context, pf_id):
LOG.debug(_("deactivate_packet_filter(): skip, "
"Not found OFC Mapping for packet_filter id=%s."),
pf_id)
return packet_filter
if pf_status != current:
self._update_resource_status(context, "packet_filter", pf_id,
pf_status)
packet_filter.update({'status': pf_status})
return packet_filter
LOG.debug(_("deactivate_packet_filter(): "
"deleting packet_filter id=%s from OFC."), pf_id)
try:
self.ofc.delete_ofc_packet_filter(context, pf_id)
self._update_resource_status_if_changed(
context, "packet_filter", packet_filter, pf_db.PF_STATUS_DOWN)
return packet_filter
except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:
with excutils.save_and_reraise_exception():
LOG.error(_("Failed to delete packet_filter id=%(id)s "
"from OFC: %(exc)s"),
{'id': pf_id, 'exc': str(exc)})
self._update_resource_status_if_changed(
context, "packet_filter", packet_filter,
pf_db.PF_STATUS_ERROR)
def activate_packet_filters_by_port(self, context, port_id):
if not self.packet_filter_enabled:
@ -240,14 +235,22 @@ class PacketFilterMixin(pf_db.PacketFilterDbMixin):
for pf in pfs:
self.activate_packet_filter_if_ready(context, pf)
def deactivate_packet_filters_by_port(self, context, port_id):
def deactivate_packet_filters_by_port(self, context, port_id,
raise_exc=True):
if not self.packet_filter_enabled:
return
filters = {'in_port': [port_id], 'status': [pf_db.PF_STATUS_ACTIVE]}
pfs = self.get_packet_filters(context, filters=filters)
error = False
for pf in pfs:
self.deactivate_packet_filter(context, pf)
try:
self.deactivate_packet_filter(context, pf)
except (nexc.OFCException, nexc.OFCMappingNotFound):
error = True
if raise_exc and error:
raise nexc.OFCException(_('Error occurred while disabling packet '
'filter(s) for port %s'), port_id)
def get_packet_filters_for_port(self, context, port):
if self.packet_filter_enabled:

2
neutron/plugins/nec/router_drivers.py
View File

@ -162,6 +162,8 @@ class RouterOpenFlowDriver(RouterDriverBase):
@call_log.log
def delete_router(self, context, router_id, router):
if not self.ofc.exists_ofc_router(context, router_id):
return
try:
self.ofc.delete_ofc_router(context, router_id, router)
except (nexc.OFCException, nexc.OFCMappingNotFound) as exc:

61
neutron/tests/unit/nec/test_nec_plugin.py
View File

@ -361,6 +361,7 @@ class TestNecPluginOfcManager(NecPluginV2TestCase):
mock.call.create_ofc_tenant(ctx, self._tenant_id),
mock.call.create_ofc_network(ctx, self._tenant_id, net['id'],
net['name']),
mock.call.exists_ofc_network(ctx, net['id']),
mock.call.delete_ofc_network(ctx, net['id'], mock.ANY),
mock.call.exists_ofc_tenant(ctx, self._tenant_id),
mock.call.delete_ofc_tenant(ctx, self._tenant_id)
@ -379,6 +380,7 @@ class TestNecPluginOfcManager(NecPluginV2TestCase):
mock.call.create_ofc_tenant(ctx, self._tenant_id),
mock.call.create_ofc_network(ctx, self._tenant_id, net['id'],
net['name']),
mock.call.exists_ofc_network(ctx, net['id']),
mock.call.delete_ofc_network(ctx, net['id'], mock.ANY),
mock.call.exists_ofc_tenant(ctx, self._tenant_id),
mock.call.delete_ofc_tenant(ctx, self._tenant_id)
@ -403,7 +405,9 @@ class TestNecPluginOfcManager(NecPluginV2TestCase):
mock.call.exists_ofc_tenant(ctx, self._tenant_id),
mock.call.create_ofc_network(ctx, self._tenant_id, nets[1]['id'],
nets[1]['name']),
mock.call.exists_ofc_network(ctx, nets[1]['id']),
mock.call.delete_ofc_network(ctx, nets[1]['id'], mock.ANY),
mock.call.exists_ofc_network(ctx, nets[0]['id']),
mock.call.delete_ofc_network(ctx, nets[0]['id'], mock.ANY),
mock.call.exists_ofc_tenant(ctx, self._tenant_id),
mock.call.delete_ofc_tenant(ctx, self._tenant_id)
@ -467,6 +471,7 @@ class TestNecPluginOfcManager(NecPluginV2TestCase):
mock.call.create_ofc_tenant(ctx, self._tenant_id),
mock.call.create_ofc_network(ctx, self._tenant_id, net['id'],
net['name']),
mock.call.exists_ofc_network(ctx, net['id']),
mock.call.delete_ofc_network(ctx, net['id'], mock.ANY),
mock.call.exists_ofc_tenant(ctx, self._tenant_id),
mock.call.delete_ofc_tenant(ctx, self._tenant_id)
@ -495,6 +500,7 @@ class TestNecPluginOfcManager(NecPluginV2TestCase):
net['name']),
mock.call.exists_ofc_port(ctx, p1['id']),
mock.call.exists_ofc_network(ctx, net['id']),
mock.call.delete_ofc_network(ctx, net['id'], mock.ANY),
mock.call.exists_ofc_tenant(ctx, self._tenant_id),
mock.call.delete_ofc_tenant(ctx, self._tenant_id)
@ -538,6 +544,7 @@ class TestNecPluginOfcManager(NecPluginV2TestCase):
mock.call.exists_ofc_port(ctx, p1['id']),
mock.call.delete_ofc_port(ctx, p1['id'], mock.ANY),
mock.call.exists_ofc_network(ctx, net['id']),
mock.call.delete_ofc_network(ctx, net['id'], mock.ANY),
mock.call.exists_ofc_tenant(ctx, self._tenant_id),
mock.call.delete_ofc_tenant(ctx, self._tenant_id)
@ -570,12 +577,37 @@ class TestNecPluginOfcManager(NecPluginV2TestCase):
mock.call.create_ofc_port(ctx, p['id'], mock.ANY),
mock.call.exists_ofc_port(ctx, p['id']),
mock.call.delete_ofc_port(ctx, p['id'], mock.ANY),
mock.call.exists_ofc_network(ctx, net['id']),
mock.call.delete_ofc_network(ctx, net['id'], mock.ANY),
mock.call.exists_ofc_tenant(ctx, self._tenant_id),
mock.call.delete_ofc_tenant(ctx, self._tenant_id)
]
self.ofc.assert_has_calls(expected)
def test_delete_network_with_error_status(self):
self.ofc.set_raise_exc('create_ofc_network',
nexc.OFCException(reason='fake error'))
with self.network() as net:
net_id = net['network']['id']
net_ref = self._show('networks', net_id)
self.assertEqual(net_ref['network']['status'], 'ERROR')
ctx = mock.ANY
tenant_id = self._tenant_id
net_name = mock.ANY
net = mock.ANY
expected = [
mock.call.exists_ofc_tenant(ctx, tenant_id),
mock.call.create_ofc_tenant(ctx, tenant_id),
mock.call.create_ofc_network(ctx, tenant_id, net_id, net_name),
mock.call.exists_ofc_network(ctx, net_id),
mock.call.exists_ofc_tenant(ctx, tenant_id),
mock.call.delete_ofc_tenant(ctx, tenant_id),
]
self.ofc.assert_has_calls(expected)
self.assertFalse(self.ofc.delete_ofc_network.call_count)
def test_delete_network_with_ofc_deletion_failure(self):
self.ofc.set_raise_exc('delete_ofc_network',
nexc.OFCException(reason='hoge'))
@ -597,7 +629,9 @@ class TestNecPluginOfcManager(NecPluginV2TestCase):
net = mock.ANY
expected = [
mock.call.create_ofc_network(ctx, tenant, net_id, net_name),
mock.call.exists_ofc_network(ctx, net_id),
mock.call.delete_ofc_network(ctx, net_id, net),
mock.call.exists_ofc_network(ctx, net_id),
mock.call.delete_ofc_network(ctx, net_id, net),
]
self.ofc.assert_has_calls(expected)
@ -641,6 +675,7 @@ class TestNecPluginOfcManager(NecPluginV2TestCase):
mock.call.delete_ofc_port(ctx, port_id, port),
mock.call.exists_ofc_port(ctx, port_id),
mock.call.delete_ofc_port(ctx, port_id, port),
mock.call.exists_ofc_network(ctx, net_id),
mock.call.delete_ofc_network(ctx, net_id, net)
]
self.ofc.assert_has_calls(expected)
@ -707,6 +742,7 @@ class TestNecPluginOfcManager(NecPluginV2TestCase):
mock.call.delete_ofc_port(ctx, p1['id'], mock.ANY),
mock.call.exists_ofc_port(ctx, p1['id']),
mock.call.exists_ofc_network(ctx, net['id']),
mock.call.delete_ofc_network(ctx, net['id'], mock.ANY),
mock.call.exists_ofc_tenant(ctx, self._tenant_id),
mock.call.delete_ofc_tenant(ctx, self._tenant_id)
@ -766,8 +802,8 @@ class TestNecPluginOfcManager(NecPluginV2TestCase):
nexc.OFCException(reason='hoge'))
body = {'port': {'admin_state_up': False}}
res = self._update('ports', port_id, body)
self.assertEqual(res['port']['status'], 'ERROR')
self._update('ports', port_id, body,
expected_code=webob.exc.HTTPInternalServerError.code)
port_ref = self._show('ports', port_id)
self.assertEqual(port_ref['port']['status'], 'ERROR')
@ -799,6 +835,27 @@ class TestNecPluginOfcManager(NecPluginV2TestCase):
self.ofc.assert_has_calls(expected)
self.assertEqual(self.ofc.delete_ofc_port.call_count, 2)
def test_delete_port_with_error_status(self):
self.ofc.set_raise_exc('create_ofc_port',
nexc.OFCException(reason='fake'))
with self.port() as port:
port_id = port['port']['id']
portinfo = {'id': port_id, 'port_no': 123}
self.rpcapi_update_ports(added=[portinfo])
port_ref = self._show('ports', port_id)
self.assertEqual(port_ref['port']['status'], 'ERROR')
ctx = mock.ANY
port = mock.ANY
expected = [
mock.call.exists_ofc_port(ctx, port_id),
mock.call.create_ofc_port(ctx, port_id, port),
mock.call.exists_ofc_port(ctx, port_id),
]
self.ofc.assert_has_calls(expected)
self.assertFalse(self.ofc.delete_ofc_port.call_count)
def test_delete_port_with_ofc_deletion_failure(self):
self.ofc.set_raise_exc('delete_ofc_port',
nexc.OFCException(reason='hoge'))

28
neutron/tests/unit/nec/test_packet_filter.py
View File

@ -388,6 +388,25 @@ class TestNecPluginPacketFilter(TestNecPluginPacketFilterBase):
self.ofc.delete_ofc_packet_filter.assert_called_once_with(
ctx, pf_id)
def test_delete_pf_with_error_status(self):
self.ofc.set_raise_exc('create_ofc_packet_filter',
nexc.OFCException(reason='fake'))
with self.packet_filter_on_network() as pf:
pf_id = pf['packet_filter']['id']
pf_ref = self._show('packet_filters', pf_id)
self.assertEqual(pf_ref['packet_filter']['status'], 'ERROR')
ctx = mock.ANY
pf_dict = mock.ANY
expected = [
mock.call.exists_ofc_packet_filter(ctx, pf_id),
mock.call.create_ofc_packet_filter(ctx, pf_id, pf_dict),
mock.call.exists_ofc_packet_filter(ctx, pf_id),
]
self.ofc.assert_has_calls(expected)
self.assertEqual(1, self.ofc.create_ofc_packet_filter.call_count)
self.assertEqual(0, self.ofc.delete_ofc_packet_filter.call_count)
def test_activate_pf_on_port_triggered_by_update_port(self):
ctx = mock.ANY
pf_dict = mock.ANY
@ -437,7 +456,8 @@ class TestNecPluginPacketFilter(TestNecPluginPacketFilterBase):
# This update request will make plugin reactivate pf.
data = {'packet_filter': {'priority': 1000}}
self._update('packet_filters', pf_id, data)
self._update('packet_filters', pf_id, data,
expected_code=webob.exc.HTTPInternalServerError.code)
self.ofc.set_raise_exc('delete_ofc_packet_filter', None)
@ -451,8 +471,7 @@ class TestNecPluginPacketFilter(TestNecPluginPacketFilterBase):
mock.call.delete_ofc_packet_filter(ctx, pf_id),
mock.call.exists_ofc_packet_filter(ctx, pf_id),
mock.call.exists_ofc_packet_filter(ctx, pf_id),
mock.call.delete_ofc_packet_filter(ctx, pf_id),
]
self.ofc.assert_has_calls(expected)
self.assertEqual(self.ofc.delete_ofc_packet_filter.call_count, 2)
@ -466,7 +485,8 @@ class TestNecPluginPacketFilter(TestNecPluginPacketFilterBase):
nexc.OFCException(reason='hoge'))
data = {'packet_filter': {'admin_state_up': False}}
self._update('packet_filters', pf_id, data)
self._update('packet_filters', pf_id, data,
expected_code=webob.exc.HTTPInternalServerError.code)
pf_ref = self._show('packet_filters', pf_id)
self.assertEqual(pf_ref['packet_filter']['status'], 'ERROR')