From 566578e8fdcb3e39ccd8c733db597e71ee4303b5 Mon Sep 17 00:00:00 2001 From: Silvan Kaiser Date: Wed, 22 Nov 2017 11:41:40 +0100 Subject: [PATCH] Added Handling Newer Quobyte API Error Codes This added the capability to expect specific error codes when doing a RPC call in the Quobyte driver. This is used to handle the newer API error codes in newer (1.4+) Quobyte API versions. This also adds explicitely creating a Quobyte tenant. Closes-Bug: #1733807 Change-Id: I65e87e6f50e12bfbe5d7a8fd988ca14bddd212da (cherry picked from commit 269942101be6de85005969af6b700dbcfdccedd1) --- manila/share/drivers/quobyte/jsonrpc.py | 14 ++++--- manila/share/drivers/quobyte/quobyte.py | 10 ++++- .../share/drivers/quobyte/test_jsonrpc.py | 35 ++++++++++++++++- .../share/drivers/quobyte/test_quobyte.py | 38 +++++++++++++++---- .../qb-bug-1733807-581e71e6581de28e.yaml | 5 +++ 5 files changed, 86 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/qb-bug-1733807-581e71e6581de28e.yaml diff --git a/manila/share/drivers/quobyte/jsonrpc.py b/manila/share/drivers/quobyte/jsonrpc.py index 6b73ae2fc7..ba4b60ec62 100644 --- a/manila/share/drivers/quobyte/jsonrpc.py +++ b/manila/share/drivers/quobyte/jsonrpc.py @@ -33,6 +33,8 @@ from manila import utils LOG = log.getLogger(__name__) ERROR_ENOENT = 2 +ERROR_ENTITY_NOT_FOUND = -24 +ERROR_GARBAGE_ARGS = -3 class JsonRpc(object): @@ -57,7 +59,7 @@ class JsonRpc(object): self._cert_file = cert_file @utils.synchronized('quobyte-request') - def call(self, method_name, user_parameters): + def call(self, method_name, user_parameters, expected_errors=[]): # prepare request self._id += 1 parameters = {'retry': 'INFINITELY'} # Backend specific setting @@ -94,17 +96,19 @@ class JsonRpc(object): if result.status_code == codes['OK']: LOG.debug("Retrieved data from Quobyte backend: %s", result.text) response = result.json() - return self._checked_for_application_error(response) + return self._checked_for_application_error(response, + expected_errors) # If things did not work out provide error info LOG.debug("Backend request resulted in error: %s", result.text) result.raise_for_status() - def _checked_for_application_error(self, result): + def _checked_for_application_error(self, result, expected_errors=[]): if 'error' in result and result['error']: if 'message' in result['error'] and 'code' in result['error']: - if result["error"]["code"] == ERROR_ENOENT: - return None # No Entry + if result["error"]["code"] in expected_errors: + # hit an expected error, return empty result + return None else: raise exception.QBRpcException( result=result["error"]["message"], diff --git a/manila/share/drivers/quobyte/quobyte.py b/manila/share/drivers/quobyte/quobyte.py index f6dd62896c..4dac9a6e78 100644 --- a/manila/share/drivers/quobyte/quobyte.py +++ b/manila/share/drivers/quobyte/quobyte.py @@ -76,9 +76,10 @@ class QuobyteShareDriver(driver.ExecuteMixin, driver.ShareDriver,): 1.2.1 - Improved capacity calculation 1.2.2 - Minor optimizations 1.2.3 - Updated RPC layer for improved stability + 1.2.4 - Fixed handling updated QB API error codes """ - DRIVER_VERSION = '1.2.3' + DRIVER_VERSION = '1.2.4' def __init__(self, *args, **kwargs): super(QuobyteShareDriver, self).__init__(False, *args, **kwargs) @@ -190,7 +191,8 @@ class QuobyteShareDriver(driver.ExecuteMixin, driver.ShareDriver,): """Resolve a volume name to the global volume uuid.""" result = self.rpc.call('resolveVolumeName', dict( volume_name=volume_name, - tenant_domain=tenant_domain)) + tenant_domain=tenant_domain), [jsonrpc.ERROR_ENOENT, + jsonrpc.ERROR_ENTITY_NOT_FOUND]) if result: return result['volume_uuid'] return None # not found @@ -223,6 +225,10 @@ class QuobyteShareDriver(driver.ExecuteMixin, driver.ShareDriver,): self._get_project_name(context, share['project_id'])) if not volume_uuid: + # create tenant, expect ERROR_GARBAGE_ARGS if it already exists + self.rpc.call('setTenant', + dict(tenant=dict(tenant_id=share['project_id'])), + expected_errors=[jsonrpc.ERROR_GARBAGE_ARGS]) result = self.rpc.call('createVolume', dict( name=share['name'], tenant_domain=share['project_id'], diff --git a/manila/tests/share/drivers/quobyte/test_jsonrpc.py b/manila/tests/share/drivers/quobyte/test_jsonrpc.py index bc5f06f67f..3e869e5df5 100644 --- a/manila/tests/share/drivers/quobyte/test_jsonrpc.py +++ b/manila/tests/share/drivers/quobyte/test_jsonrpc.py @@ -141,6 +141,29 @@ class QuobyteJsonRpcTestCase(test.TestCase): verify=fake_ca_file) self.assertEqual("Sweet gorilla of Manila", result) + @mock.patch.object(jsonrpc.JsonRpc, "_checked_for_application_error", + return_value="Sweet gorilla of Manila") + @mock.patch.object(requests, "post", + return_value=FakeResponse( + 200, {"result": "Sweet gorilla of Manila"})) + def test_https_call_verify_expected_error(self, mock_req_get, mock_check): + fake_ca_file = tempfile.TemporaryFile() + self.rpc = jsonrpc.JsonRpc(url="https://test", + user_credentials=("me", "team"), + ca_file=fake_ca_file) + + result = self.rpc.call('method', {'param': 'value'}, + expected_errors=[42]) + + mock_req_get.assert_called_once_with( + url=self.rpc._url, + json=mock.ANY, # not checking here as of undefined order in dict + auth=self.rpc._credentials, + verify=fake_ca_file) + mock_check.assert_called_once_with( + {'result': 'Sweet gorilla of Manila'}, [42]) + self.assertEqual("Sweet gorilla of Manila", result) + @mock.patch.object(requests, "post", side_effect=exceptions.HTTPError) def test_jsonrpc_call_http_exception(self, req_get_mock): self.assertRaises(exceptions.HTTPError, @@ -169,12 +192,22 @@ class QuobyteJsonRpcTestCase(test.TestCase): (self.rpc._checked_for_application_error( result=resultdict))) + def test_checked_for_application_error_enf(self): + resultdict = {"result": "Sweet gorilla of Manila", + "error": {"message": "No Gorilla", + "code": jsonrpc.ERROR_ENTITY_NOT_FOUND}} + self.assertIsNone( + self.rpc._checked_for_application_error( + result=resultdict, + expected_errors=[jsonrpc.ERROR_ENTITY_NOT_FOUND])) + def test_checked_for_application_error_no_entry(self): resultdict = {"result": "Sweet gorilla of Manila", "error": {"message": "No Gorilla", "code": jsonrpc.ERROR_ENOENT}} self.assertIsNone( - self.rpc._checked_for_application_error(result=resultdict)) + self.rpc._checked_for_application_error( + result=resultdict, expected_errors=[jsonrpc.ERROR_ENOENT])) def test_checked_for_application_error_exception(self): self.assertRaises(exception.QBRpcException, diff --git a/manila/tests/share/drivers/quobyte/test_quobyte.py b/manila/tests/share/drivers/quobyte/test_quobyte.py index 9d9df102cc..4914513a17 100644 --- a/manila/tests/share/drivers/quobyte/test_quobyte.py +++ b/manila/tests/share/drivers/quobyte/test_quobyte.py @@ -29,7 +29,7 @@ from manila.tests import fake_share CONF = cfg.CONF -def fake_rpc_handler(name, *args): +def fake_rpc_handler(name, *args, **kwargs): if name == 'resolveVolumeName': return None elif name == 'createVolume': @@ -136,8 +136,23 @@ class QuobyteShareDriverTestCase(test.TestCase): self._driver.create_share(self._context, self.share) - self._driver.rpc.call.assert_called_with( - 'exportVolume', dict(protocol='NFS', volume_uuid='voluuid')) + resolv_params = {'tenant_domain': 'fake_project_uuid', + 'volume_name': 'fakename'} + sett_params = {'tenant': {'tenant_id': 'fake_project_uuid'}} + create_params = dict( + name='fakename', + tenant_domain='fake_project_uuid', + root_user_id='root', + root_group_id='root', + configuration_name='BASE') + self._driver.rpc.call.assert_has_calls([ + mock.call('resolveVolumeName', resolv_params, + [jsonrpc.ERROR_ENOENT, jsonrpc.ERROR_ENTITY_NOT_FOUND]), + mock.call('setTenant', sett_params, + expected_errors=[jsonrpc.ERROR_GARBAGE_ARGS]), + mock.call('createVolume', create_params), + mock.call('exportVolume', dict(protocol='NFS', + volume_uuid='voluuid'))]) def test_create_share_wrong_protocol(self): share = {'share_proto': 'WRONG_PROTOCOL'} @@ -162,7 +177,8 @@ class QuobyteShareDriverTestCase(test.TestCase): resolv_params = {'volume_name': 'fakename', 'tenant_domain': 'fake_project_uuid'} self._driver.rpc.call.assert_has_calls([ - mock.call('resolveVolumeName', resolv_params), + mock.call('resolveVolumeName', resolv_params, + [jsonrpc.ERROR_ENOENT, jsonrpc.ERROR_ENTITY_NOT_FOUND]), mock.call('deleteVolume', {'volume_uuid': 'voluuid'})]) def test_delete_share_existing_volume_disabled(self): @@ -178,8 +194,7 @@ class QuobyteShareDriverTestCase(test.TestCase): self._driver.delete_share(self._context, self.share) self._driver.rpc.call.assert_called_with( - 'exportVolume', {'volume_uuid': 'voluuid', - 'remove_export': True}) + 'exportVolume', {'volume_uuid': 'voluuid', 'remove_export': True}) @mock.patch.object(quobyte.LOG, 'warning') def test_delete_share_nonexisting_volume(self, mock_warning): @@ -277,8 +292,9 @@ class QuobyteShareDriverTestCase(test.TestCase): exp_params = {'volume_name': 'fake_vol_name', 'tenant_domain': 'fake_domain_name'} - self._driver.rpc.call.assert_called_with('resolveVolumeName', - exp_params) + self._driver.rpc.call.assert_called_with( + 'resolveVolumeName', exp_params, + [jsonrpc.ERROR_ENOENT, jsonrpc.ERROR_ENTITY_NOT_FOUND]) def test_resolve_volume_name_NOENT(self): self._driver.rpc.call = mock.Mock( @@ -287,6 +303,12 @@ class QuobyteShareDriverTestCase(test.TestCase): self.assertIsNone( self._driver._resolve_volume_name('fake_vol_name', 'fake_domain_name')) + self._driver.rpc.call.assert_called_once_with( + 'resolveVolumeName', + dict(volume_name='fake_vol_name', + tenant_domain='fake_domain_name'), + [jsonrpc.ERROR_ENOENT, jsonrpc.ERROR_ENTITY_NOT_FOUND] + ) def test_resolve_volume_name_other_error(self): self._driver.rpc.call = mock.Mock( diff --git a/releasenotes/notes/qb-bug-1733807-581e71e6581de28e.yaml b/releasenotes/notes/qb-bug-1733807-581e71e6581de28e.yaml new file mode 100644 index 0000000000..c3363c6836 --- /dev/null +++ b/releasenotes/notes/qb-bug-1733807-581e71e6581de28e.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + The Quobyte driver now handles updated error codes from Quobyte API + versions 1.4+ .