x/vmware-nsx
Code Issues Proposed changes

nec plugin: allow to delete resource with ERROR status

Browse Source 
Previously if a resource is in ERROR status and there is no
corresponding resource on OpenFlow controller, the resource
cannot be deleted through an API request.
This commit rearrange ERROR status check to allow resource
with ERROR status to be deleted.

Closes-Bug: #1295754
Change-Id: I709f5e2066eb5d12ec0f42dff15797acddc2009e
This commit is contained in:
Akihiro Motoki 2014-03-16 00:16:23 +09:00
parent 1ed0b46373
commit cff764d496
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)
@ -627,10 +637,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:
@ -738,9 +746,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', []):
@ -761,8 +770,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')