From 49e953a8ef9d552e9c5be1e2e807634c68af5c5f Mon Sep 17 00:00:00 2001 From: Ryota MIBU Date: Mon, 29 Jul 2013 22:07:19 +0900 Subject: [PATCH] Make NEC Plugin keep error resources NEC Plugin used to ignore OFC errors while deleting resources from OFC, and it could leave some unused resources on OFC. If OFC generates id which is in remained resources when creating new resource, it will fail. This commit makes NEC Plugin keep logical resource when it failed to delete resource from OFC not to leave orphan resource on OFC, and raise exception to tell the user that the resouce status is Error. NOTE: The user can retry deletion. If the resouce was successfully deleted from OFC in retries, the logical resource will be deleted. Fixes: bug #1206416 Change-Id: Ifea38dfe3fe8b18d7ae1cedf86a23008549250cc --- neutron/plugins/nec/nec_plugin.py | 43 ++++++--- neutron/tests/unit/nec/test_nec_plugin.py | 103 ++++++++++++++++++++++ 2 files changed, 133 insertions(+), 13 deletions(-) diff --git a/neutron/plugins/nec/nec_plugin.py b/neutron/plugins/nec/nec_plugin.py index 17c0e62a8..7f9b97c97 100644 --- a/neutron/plugins/nec/nec_plugin.py +++ b/neutron/plugins/nec/nec_plugin.py @@ -218,6 +218,7 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2, if port_status is not port['status']: self._update_resource_status(context, "port", port['id'], port_status) + port['status'] = port_status # deactivate packet_filters after the port has deleted from OFC. if self.packet_filter_enabled: @@ -227,6 +228,8 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2, for pf in pfs: self.deactivate_packet_filter(context, pf) + return port + # Quantm Plugin Basic methods def create_network(self, context, network): @@ -309,15 +312,25 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2, LOG.debug(_("NECPluginV2.delete_network() called, id=%s ."), id) net = super(NECPluginV2, self).get_network(context, id) tenant_id = net['tenant_id'] + ports = self.get_ports(context, filters={'network_id': [id]}) + + # check if there are any tenant owned ports in-use + only_auto_del = all(p['device_owner'] in + db_base_plugin_v2.AUTO_DELETE_PORT_OWNERS + for p in ports) + if not only_auto_del: + raise q_exc.NetworkInUse(net_id=id) # Make sure auto-delete ports on OFC are deleted. - filter = {'network_id': [id], - 'device_owner': db_base_plugin_v2.AUTO_DELETE_PORT_OWNERS} - auto_delete_ports = self.get_ports(context, filters=filter) - for port in auto_delete_ports: - LOG.debug(_('delete_network(): deleting auto-delete port' - ' from OFC: %s'), port) - self.deactivate_port(context, port) + _error_ports = [] + for port in ports: + port = self.deactivate_port(context, port) + if port['status'] == OperationalStatus.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 if self.packet_filter_enabled: @@ -326,16 +339,17 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2, for pf in pfs: self.delete_packet_filter(context, pf['id']) - super(NECPluginV2, self).delete_network(context, id) try: # 'net' parameter is required to lookup old OFC mapping self.ofc.delete_ofc_network(context, id, net) except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc: reason = _("delete_network() failed due to %s") % exc - # NOTE: The OFC configuration of this network could be remained - # as an orphan resource. But, it does NOT harm any other - # resources, so this plugin just warns. - LOG.warn(reason) + LOG.error(reason) + self._update_resource_status(context, "network", net['id'], + OperationalStatus.ERROR) + raise + + super(NECPluginV2, self).delete_network(context, id) # delete unnessary ofc_tenant filters = dict(tenant_id=[tenant_id]) @@ -407,7 +421,10 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2, # Thus we need to call self.get_port() instead of super().get_port() port = self.get_port(context, id) - self.deactivate_port(context, port) + port = self.deactivate_port(context, port) + if port['status'] == OperationalStatus.ERROR: + reason = _("Failed to delete port=%s from OFC.") % id + raise nexc.OFCException(reason=reason) # delete all packet_filters of the port if self.packet_filter_enabled: diff --git a/neutron/tests/unit/nec/test_nec_plugin.py b/neutron/tests/unit/nec/test_nec_plugin.py index c9cd5ee52..bab4f3d33 100644 --- a/neutron/tests/unit/nec/test_nec_plugin.py +++ b/neutron/tests/unit/nec/test_nec_plugin.py @@ -17,10 +17,12 @@ import os import fixtures import mock +import webob.exc from neutron.common.test_lib import test_config from neutron.common import topics from neutron import context +from neutron.db import db_base_plugin_v2 from neutron.extensions import portbindings from neutron import manager from neutron.plugins.nec.common import exceptions as nexc @@ -530,6 +532,76 @@ class TestNecPluginOfcManager(NecPluginV2TestCase): ] self.ofc.assert_has_calls(expected) + def test_delete_network_with_ofc_deletion_failure(self): + self.ofc.set_raise_exc('delete_ofc_network', + nexc.OFCException(reason='hoge')) + + with self.network() as net: + net_id = net['network']['id'] + + self._delete('networks', net_id, + expected_code=webob.exc.HTTPInternalServerError.code) + + net_ref = self._show('networks', net_id) + self.assertEqual(net_ref['network']['status'], 'ERROR') + + self.ofc.set_raise_exc('delete_ofc_network', None) + + ctx = mock.ANY + tenant = mock.ANY + net_name = mock.ANY + net = mock.ANY + expected = [ + mock.call.create_ofc_network(ctx, tenant, net_id, net_name), + mock.call.delete_ofc_network(ctx, net_id, net), + mock.call.delete_ofc_network(ctx, net_id, net), + ] + self.ofc.assert_has_calls(expected) + self.assertEqual(self.ofc.delete_ofc_network.call_count, 2) + + def test_delete_network_with_deactivating_auto_delete_port_failure(self): + self.ofc.set_raise_exc('delete_ofc_port', + nexc.OFCException(reason='hoge')) + + with self.network(do_delete=False) as net: + net_id = net['network']['id'] + + device_owner = db_base_plugin_v2.AUTO_DELETE_PORT_OWNERS[0] + port = self._make_port(self.fmt, net_id, device_owner=device_owner) + port_id = port['port']['id'] + + portinfo = {'id': port_id, 'port_no': 123} + self.rpcapi_update_ports(added=[portinfo]) + + self._delete('networks', net_id, + expected_code=webob.exc.HTTPInternalServerError.code) + + net_ref = self._show('networks', net_id) + self.assertEqual(net_ref['network']['status'], 'ACTIVE') + port_ref = self._show('ports', port_id) + self.assertEqual(port_ref['port']['status'], 'ERROR') + + self.ofc.set_raise_exc('delete_ofc_port', None) + self._delete('networks', net_id) + + ctx = mock.ANY + tenant = mock.ANY + net_name = mock.ANY + net = mock.ANY + port = mock.ANY + expected = [ + mock.call.create_ofc_network(ctx, tenant, net_id, net_name), + 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), + 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.delete_ofc_network(ctx, net_id, net) + ] + self.ofc.assert_has_calls(expected) + self.assertEqual(self.ofc.delete_ofc_network.call_count, 1) + def test_update_port(self): self._test_update_port_with_admin_state(resource='port') @@ -592,3 +664,34 @@ class TestNecPluginOfcManager(NecPluginV2TestCase): mock.call.delete_ofc_tenant(ctx, self._tenant_id) ] self.ofc.assert_has_calls(expected) + + def test_delete_port_with_ofc_deletion_failure(self): + self.ofc.set_raise_exc('delete_ofc_port', + nexc.OFCException(reason='hoge')) + + with self.port() as port: + port_id = port['port']['id'] + + portinfo = {'id': port_id, 'port_no': 123} + self.rpcapi_update_ports(added=[portinfo]) + + self._delete('ports', port_id, + expected_code=webob.exc.HTTPInternalServerError.code) + + port_ref = self._show('ports', port_id) + self.assertEqual(port_ref['port']['status'], 'ERROR') + + self.ofc.set_raise_exc('delete_ofc_port', None) + + 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), + 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) + ] + self.ofc.assert_has_calls(expected) + self.assertEqual(self.ofc.delete_ofc_port.call_count, 2) -- 2.45.2