From 81a654fdebab13639d2d4ffacfc254b5893bbe7f Mon Sep 17 00:00:00 2001 From: armando-migliaccio Date: Fri, 9 Aug 2013 16:27:22 -0700 Subject: [PATCH] Returns 503 if the NVP cluster is in maintenance mode If the NVP cluster is in 'readonly-mode' during a maintenance window, some NVP operations may raise a Forbidden error. This is not currently handled correctly, and Neutron server ends up returning 500. This patch addresses the problem by ensuring the right error code is returned. Fixes bug 1204715 Change-Id: Ibd2cac8286a0978a95f9006142c2a405053dfa00 --- neutron/common/exceptions.py | 2 +- neutron/plugins/nicira/NeutronPlugin.py | 4 +- neutron/plugins/nicira/NvpApiClient.py | 21 ++++++--- neutron/plugins/nicira/common/exceptions.py | 6 +++ neutron/plugins/nicira/nvplib.py | 2 + .../tests/unit/nicira/test_nicira_plugin.py | 45 +++++++++++++++++++ neutron/tests/unit/nicira/test_nvplib.py | 11 +++-- 7 files changed, 79 insertions(+), 12 deletions(-) diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py index 65c05af433..ae58e71dfc 100644 --- a/neutron/common/exceptions.py +++ b/neutron/common/exceptions.py @@ -59,7 +59,7 @@ class NotAuthorized(NeutronException): class ServiceUnavailable(NeutronException): - message = _("The service is unailable") + message = _("The service is unavailable") class AdminRequired(NotAuthorized): diff --git a/neutron/plugins/nicira/NeutronPlugin.py b/neutron/plugins/nicira/NeutronPlugin.py index c8f84e031c..0ffc407ba4 100644 --- a/neutron/plugins/nicira/NeutronPlugin.py +++ b/neutron/plugins/nicira/NeutronPlugin.py @@ -757,7 +757,9 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, base.FAULT_MAP.update({nvp_exc.NvpInvalidNovaZone: webob.exc.HTTPBadRequest, nvp_exc.NvpNoMorePortsException: - webob.exc.HTTPBadRequest}) + webob.exc.HTTPBadRequest, + nvp_exc.MaintenanceInProgress: + webob.exc.HTTPServiceUnavailable}) def _handle_provider_create(self, context, attrs): # NOTE(salvatore-orlando): This method has been borrowed from diff --git a/neutron/plugins/nicira/NvpApiClient.py b/neutron/plugins/nicira/NvpApiClient.py index d64c372165..b7c32e65e2 100644 --- a/neutron/plugins/nicira/NvpApiClient.py +++ b/neutron/plugins/nicira/NvpApiClient.py @@ -147,7 +147,7 @@ class NVPApiHelper(client_eventlet.NvpApiClientEventlet): if status in self.error_codes: LOG.error(_("Received error code: %s"), status) LOG.error(_("Server Error Message: %s"), response.body) - self.error_codes[status](self) + self.error_codes[status](self, response) # Continue processing for non-error condition. if (status != httplib.OK and status != httplib.CREATED @@ -174,19 +174,22 @@ class NVPApiHelper(client_eventlet.NvpApiClientEventlet): 'Plugin might not work as expected.')) return self._nvp_version - def fourZeroFour(self): + def fourZeroFour(self, response=None): raise ResourceNotFound() - def fourZeroNine(self): + def fourZeroNine(self, response=None): raise Conflict() - def fiveZeroThree(self): + def fiveZeroThree(self, response=None): raise ServiceUnavailable() - def fourZeroThree(self): - raise Forbidden() + def fourZeroThree(self, response=None): + if 'read-only' in response.body: + raise ReadOnlyMode() + else: + raise Forbidden() - def zero(self): + def zero(self, response=None): raise NvpApiException() # TODO(del): ensure error_codes are handled/raised appropriately @@ -247,5 +250,9 @@ class Forbidden(NvpApiException): "referenced resource.") +class ReadOnlyMode(Forbidden): + message = _("Create/Update actions are forbidden when in read-only mode.") + + class RequestTimeout(NvpApiException): message = _("The request has timed out.") diff --git a/neutron/plugins/nicira/common/exceptions.py b/neutron/plugins/nicira/common/exceptions.py index 841655ca1c..21743b6d14 100644 --- a/neutron/plugins/nicira/common/exceptions.py +++ b/neutron/plugins/nicira/common/exceptions.py @@ -56,3 +56,9 @@ class NvpNatRuleMismatch(NvpPluginException): class NvpInvalidAttachmentType(NvpPluginException): message = _("Invalid NVP attachment type '%(attachment_type)s'") + + +class MaintenanceInProgress(NvpPluginException): + message = _("The networking backend is currently in maintenance mode and " + "therefore unable to accept requests which modify its state. " + "Please try later.") diff --git a/neutron/plugins/nicira/nvplib.py b/neutron/plugins/nicira/nvplib.py index 0eecd8118b..9cbf72f8d4 100644 --- a/neutron/plugins/nicira/nvplib.py +++ b/neutron/plugins/nicira/nvplib.py @@ -950,6 +950,8 @@ def do_request(*args, **kwargs): return json.loads(res) except NvpApiClient.ResourceNotFound: raise exception.NotFound() + except NvpApiClient.ReadOnlyMode: + raise nvp_exc.MaintenanceInProgress() def mk_body(**kwargs): diff --git a/neutron/tests/unit/nicira/test_nicira_plugin.py b/neutron/tests/unit/nicira/test_nicira_plugin.py index 8eeb72bd26..cdd35f1a54 100644 --- a/neutron/tests/unit/nicira/test_nicira_plugin.py +++ b/neutron/tests/unit/nicira/test_nicira_plugin.py @@ -30,6 +30,7 @@ from neutron.extensions import providernet as pnet from neutron.extensions import securitygroup as secgrp from neutron import manager from neutron.openstack.common import uuidutils +from neutron.plugins.nicira.common import exceptions as nvp_exc 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 @@ -192,6 +193,22 @@ class TestNiciraPortsV2(NiciraPluginV2TestCase, webob.exc.HTTPInternalServerError.code) self._verify_no_orphan_left(net_id) + def test_create_port_maintenance_returns_503(self): + with self.network() as net: + with mock.patch.object(nvplib, 'do_request', + side_effect=nvp_exc.MaintenanceInProgress): + data = {'port': {'network_id': net['network']['id'], + 'admin_state_up': False, + 'fixed_ips': [], + 'tenant_id': self._tenant_id}} + plugin = manager.NeutronManager.get_plugin() + with mock.patch.object(plugin, 'get_network', + return_value=net['network']): + port_req = self.new_create_request('ports', data, self.fmt) + res = port_req.get_response(self.api) + self.assertEqual(webob.exc.HTTPServiceUnavailable.code, + res.status_int) + class TestNiciraNetworksV2(test_plugin.TestNetworksV2, NiciraPluginV2TestCase): @@ -276,6 +293,17 @@ class TestNiciraNetworksV2(test_plugin.TestNetworksV2, # Assert neutron name is not truncated self.assertEqual(net['network']['name'], name) + def test_create_network_maintenance_returns_503(self): + data = {'network': {'name': 'foo', + 'admin_state_up': True, + 'tenant_id': self._tenant_id}} + with mock.patch.object(nvplib, 'do_request', + side_effect=nvp_exc.MaintenanceInProgress): + net_req = self.new_create_request('networks', data, self.fmt) + res = net_req.get_response(self.api) + self.assertEqual(webob.exc.HTTPServiceUnavailable.code, + res.status_int) + class NiciraPortSecurityTestCase(psec.PortSecurityDBTestCase): @@ -615,6 +643,23 @@ class TestNiciraL3NatTestCase(test_l3_plugin.L3NatDBTestCase, self.assertIsNone(body['floatingip']['port_id']) self.assertIsNone(body['floatingip']['fixed_ip_address']) + def test_create_router_maintenance_returns_503(self): + with self._create_l3_ext_network() as net: + with self.subnet(network=net) as s: + with mock.patch.object( + nvplib, + 'do_request', + side_effect=nvp_exc.MaintenanceInProgress): + data = {'router': {'tenant_id': 'whatever'}} + data['router']['name'] = 'router1' + data['router']['external_gateway_info'] = { + 'network_id': s['subnet']['network_id']} + router_req = self.new_create_request( + 'routers', data, self.fmt) + res = router_req.get_response(self.ext_api) + self.assertEqual(webob.exc.HTTPServiceUnavailable.code, + res.status_int) + class NvpQoSTestExtensionManager(object): diff --git a/neutron/tests/unit/nicira/test_nvplib.py b/neutron/tests/unit/nicira/test_nvplib.py index fef5b5ca86..c2c722da36 100644 --- a/neutron/tests/unit/nicira/test_nvplib.py +++ b/neutron/tests/unit/nicira/test_nvplib.py @@ -1318,7 +1318,7 @@ class TestNvplibLogicalPorts(NvplibTestCase): self.assertIn(res_port['uuid'], switch_port_uuids) -class TestNvplibClusterVersion(NvplibTestCase): +class TestNvplibClusterManagement(NvplibTestCase): def test_get_cluster_version(self): @@ -1330,7 +1330,6 @@ class TestNvplibClusterVersion(NvplibTestCase): return {'result_count': 1, 'results': [{'uuid': 'xyz'}]} - # mock do_request with mock.patch.object(nvplib, 'do_request', new=fakedorequest): version = nvplib.get_cluster_version('whatever') self.assertEqual(version, '3.0') @@ -1341,11 +1340,17 @@ class TestNvplibClusterVersion(NvplibTestCase): if 'node' in uri: return {'result_count': 0} - # mock do_request with mock.patch.object(nvplib, 'do_request', new=fakedorequest): version = nvplib.get_cluster_version('whatever') self.assertIsNone(version) + def test_cluster_in_readonly_mode(self): + with mock.patch.object(self.fake_cluster.api_client, + 'request', + side_effect=NvpApiClient.ReadOnlyMode): + self.assertRaises(nvp_exc.MaintenanceInProgress, + nvplib.do_request, cluster=self.fake_cluster) + def _nicira_method(method_name, module_name='nvplib'): return '%s.%s.%s' % ('neutron.plugins.nicira', module_name, method_name)