openstack
/
cinder
Code Issues Proposed changes

Fix: request-id missing in volume action's response header. 

Several volume actions use webob.Response object to
return HTTP codes and cinder's wsgi layer doesn't append
request-id if the actions return webob.Response object.

If we intend to return certain HTTP code, we should use
wsgi.response decorator and let cinder's wsgi layer create
response object of it's own.

Change-Id: Ibd6f60301e73022fecae4286e68d152bab73a40e
Closes-Bug: 1743404
This commit is contained in:
Ibadulla Khan 2018-01-16 12:51:10 +05:30
parent f41e556521
commit 745e84d5c4
4 changed files with 29 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
cinder/api/contrib/admin_actions.py
View File

@ -93,6 +93,7 @@ class AdminController(wsgi.Controller):
LOG.debug('Worker entry for %s with id %s has been deleted.',
self.collection, id)
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-reset_status')
def _reset_status(self, req, id, body):
"""Reset status on the resource."""
@ -126,8 +127,7 @@ class AdminController(wsgi.Controller):
notifier.info(context, self.collection + '.reset_status.end',
notifier_info)
return webob.Response(status_int=http_client.ACCEPTED)
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-force_delete')
def _force_delete(self, req, id, body):
"""Delete a resource, bypassing the check that it must be available."""
@ -136,7 +136,6 @@ class AdminController(wsgi.Controller):
# Not found exception will be handled at the wsgi level
resource = self._get(context, id)
self._delete(context, resource, force=True)
return webob.Response(status_int=http_client.ACCEPTED)
class VolumeAdminController(AdminController):
@ -200,6 +199,7 @@ class VolumeAdminController(AdminController):
"or 'migration_status' for update."))
return update
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-force_detach')
def _force_detach(self, req, id, body):
"""Roll back a bad detach after the volume been disconnected."""
@ -236,8 +236,8 @@ class VolumeAdminController(AdminController):
# be exposed to the user and in such cases it should raise
# 500 error.
raise
return webob.Response(status_int=http_client.ACCEPTED)
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-migrate_volume')
def _migrate_volume(self, req, id, body):
"""Migrate a volume to the specified host."""
@ -253,7 +253,6 @@ class VolumeAdminController(AdminController):
lock_volume = utils.get_bool_param('lock_volume', params)
self.volume_api.migrate_volume(context, volume, host, cluster_name,
force_host_copy, lock_volume)
return webob.Response(status_int=http_client.ACCEPTED)
@wsgi.action('os-migrate_volume_completion')
def _migrate_volume_completion(self, req, id, body):
@ -312,6 +311,7 @@ class BackupAdminController(AdminController):
def _delete(self, *args, **kwargs):
return self.backup_api.delete(*args, **kwargs)
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-reset_status')
def _reset_status(self, req, id, body):
"""Reset status on the resource."""
@ -330,7 +330,6 @@ class BackupAdminController(AdminController):
# Not found exception will be handled at the wsgi level
self.backup_api.reset_status(context=context, backup_id=id,
status=update['status'])
return webob.Response(status_int=http_client.ACCEPTED)
class Admin_actions(extensions.ExtensionDescriptor):

2
cinder/api/contrib/types_extra_specs.py
View File

@ -155,6 +155,7 @@ class VolumeTypeExtraSpecsController(wsgi.Controller):
raise exception.VolumeTypeExtraSpecsNotFound(
volume_type_id=type_id, extra_specs_key=id)
@wsgi.response(http_client.ACCEPTED)
def delete(self, req, type_id, id):
"""Deletes an existing extra spec."""
context = req.environ['cinder.context']
@ -175,7 +176,6 @@ class VolumeTypeExtraSpecsController(wsgi.Controller):
notifier.info(context,
'volume_type_extra_specs.delete',
notifier_info)
return webob.Response(status_int=http_client.ACCEPTED)
def _check_key_names(self, keys):
if not common.validate_key_names(keys):

22
cinder/api/contrib/volume_actions.py
View File

@ -50,6 +50,7 @@ class VolumeActionsController(wsgi.Controller):
return self._key_mgr
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-attach')
def _attach(self, req, id, body):
"""Add attachment metadata."""
@ -98,8 +99,7 @@ class VolumeActionsController(wsgi.Controller):
# to the user and in such cases it should raise 500 error.
raise
return webob.Response(status_int=http_client.ACCEPTED)
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-detach')
def _detach(self, req, id, body):
"""Clear attachment metadata."""
@ -125,8 +125,7 @@ class VolumeActionsController(wsgi.Controller):
# to the user and in such cases it should raise 500 error.
raise
return webob.Response(status_int=http_client.ACCEPTED)
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-reserve')
def _reserve(self, req, id, body):
"""Mark volume as reserved."""
@ -135,8 +134,8 @@ class VolumeActionsController(wsgi.Controller):
volume = self.volume_api.get(context, id)
self.volume_api.reserve_volume(context, volume)
return webob.Response(status_int=http_client.ACCEPTED)
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-unreserve')
def _unreserve(self, req, id, body):
"""Unmark volume as reserved."""
@ -145,8 +144,8 @@ class VolumeActionsController(wsgi.Controller):
volume = self.volume_api.get(context, id)
self.volume_api.unreserve_volume(context, volume)
return webob.Response(status_int=http_client.ACCEPTED)
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-begin_detaching')
def _begin_detaching(self, req, id, body):
"""Update volume status to 'detaching'."""
@ -155,8 +154,8 @@ class VolumeActionsController(wsgi.Controller):
volume = self.volume_api.get(context, id)
self.volume_api.begin_detaching(context, volume)
return webob.Response(status_int=http_client.ACCEPTED)
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-roll_detaching')
def _roll_detaching(self, req, id, body):
"""Roll back volume status to 'in-use'."""
@ -165,7 +164,6 @@ class VolumeActionsController(wsgi.Controller):
volume = self.volume_api.get(context, id)
self.volume_api.roll_detaching(context, volume)
return webob.Response(status_int=http_client.ACCEPTED)
@wsgi.action('os-initialize_connection')
def _initialize_connection(self, req, id, body):
@ -195,6 +193,7 @@ class VolumeActionsController(wsgi.Controller):
return {'connection_info': info}
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-terminate_connection')
def _terminate_connection(self, req, id, body):
"""Terminate volume attachment."""
@ -211,7 +210,6 @@ class VolumeActionsController(wsgi.Controller):
except exception.VolumeBackendAPIException:
msg = _("Unable to terminate volume connection from backend.")
raise webob.exc.HTTPInternalServerError(explanation=msg)
return webob.Response(status_int=http_client.ACCEPTED)
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-volume_upload_image')
@ -320,6 +318,7 @@ class VolumeActionsController(wsgi.Controller):
except exception.InvalidVolume as error:
raise webob.exc.HTTPBadRequest(explanation=error.msg)
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-update_readonly_flag')
def _volume_readonly_update(self, req, id, body):
"""Update volume readonly flag."""
@ -342,8 +341,8 @@ class VolumeActionsController(wsgi.Controller):
raise webob.exc.HTTPBadRequest(explanation=msg)
self.volume_api.update_readonly_flag(context, volume, readonly_flag)
return webob.Response(status_int=http_client.ACCEPTED)
@wsgi.response(http_client.ACCEPTED)
@wsgi.action('os-retype')
def _retype(self, req, id, body):
"""Change type of existing volume."""
@ -357,8 +356,8 @@ class VolumeActionsController(wsgi.Controller):
policy = body['os-retype'].get('migration_policy')
self.volume_api.retype(context, volume, new_type, policy)
return webob.Response(status_int=http_client.ACCEPTED)
@wsgi.response(http_client.OK)
@wsgi.action('os-set_bootable')
def _set_bootable(self, req, id, body):
"""Update bootable status of a volume."""
@ -383,7 +382,6 @@ class VolumeActionsController(wsgi.Controller):
update_dict = {'bootable': bootable}
self.volume_api.update(context, volume, update_dict)
return webob.Response(status_int=http_client.OK)
class Volume_actions(extensions.ExtensionDescriptor):

19
cinder/tests/unit/api/contrib/test_admin_actions.py
View File

@ -53,6 +53,13 @@ def app():
return mapper
def app_v3():
api = fakes.router.APIRouter()
mapper = fakes.urlmap.URLMap()
mapper['/v3'] = api
return mapper
class BaseAdminTest(test.TestCase):
def setUp(self):
super(BaseAdminTest, self).setUp()
@ -528,17 +535,17 @@ class AdminActionsTest(BaseAdminTest):
req = webob.Request.blank('/v3/%s/volumes/%s/action' % (
fake.PROJECT_ID, volume['id']))
req.method = 'POST'
req.headers['content-type'] = 'application/json'
body = {'os-migrate_volume': {'host': host,
'force_host_copy': force_host_copy}}
version = version or mv.BASE_VERSION
req.headers = mv.get_mv_header(version)
req.headers['Content-Type'] = 'application/json'
req.api_version_request = mv.get_api_version(version)
if version == mv.VOLUME_MIGRATE_CLUSTER:
body['os-migrate_volume']['cluster'] = cluster
req.body = jsonutils.dump_as_bytes(body)
req.environ['cinder.context'] = ctx
resp = self.controller._migrate_volume(req, volume.id, body)
resp = req.get_response(app_v3())
# verify status
self.assertEqual(expected_status, resp.status_int)
@ -573,10 +580,10 @@ class AdminActionsTest(BaseAdminTest):
host = 'test2'
cluster = 'cluster'
volume = self._migrate_volume_prep()
self.assertRaises(exception.InvalidInput,
self._migrate_volume_3_exec, self.ctx, volume, host,
None, version=mv.VOLUME_MIGRATE_CLUSTER,
cluster=cluster)
expected_status = http_client.BAD_REQUEST
self._migrate_volume_3_exec(self.ctx, volume, host, expected_status,
version=mv.VOLUME_MIGRATE_CLUSTER,
cluster=cluster)
def _migrate_volume_exec(self, ctx, volume, host, expected_status,
force_host_copy=False):