openstack/cinder
Code Issues Proposed changes

[BugFix] Add 'all_tenants', 'project_id' in attachment-list

Browse Source 
There are some issues around new attach/detach API/CLI,
fix them step by step. This patch adds 'all_tenants',
'project_id' support in attachment-list along with these
modifications:

1. Don't throw errors when have invalid filters and
   don't check filters when is admin context.
2. Don't apply filters again when query already applied
   limit and offset.

Change-Id: I5f469c809dfb16e52d6298c4aa13a30c27ee56f9
Closes-Bug: #1675974
This commit is contained in:
TommyLike 2017-03-25 22:37:40 +08:00
parent 5f66f158bf
commit 9bb85f24de
4 changed files with 61 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
cinder/api/v3/attachments.py
View File

@ -69,23 +69,21 @@ class AttachmentsController(wsgi.Controller):
search_opts = req.GET.copy()
sort_keys, sort_dirs = common.get_sort_params(search_opts)
marker, limit, offset = common.get_pagination_params(search_opts)
filters = search_opts
allowed = self.allowed_filters
if not allowed.issuperset(filters):
invalid_keys = set(filters).difference(allowed)
msg = _('Invalid filter keys: %s') % ', '.join(invalid_keys)
raise exception.InvalidInput(reason=msg)
# Filter out invalid options
allowed_search_options = ('status', 'volume_id',
'instance_id')
if search_opts.get('instance_id', None):
search_opts['instance_uuid'] = search_opts.get('instance_id')
utils.remove_invalid_filter_options(context, search_opts,
allowed_search_options)
return objects.VolumeAttachmentList.get_all(
context, search_opts=search_opts, marker=marker, limit=limit,
offset=offset, sort_keys=sort_keys, sort_direction=sort_dirs)
self.allowed_filters)
if context.is_admin and 'all_tenants' in search_opts:
del search_opts['all_tenants']
return objects.VolumeAttachmentList.get_all(
context, search_opts=search_opts, marker=marker, limit=limit,
offset=offset, sort_keys=sort_keys, sort_direction=sort_dirs)
else:
return objects.VolumeAttachmentList.get_all_by_project(
context, context.project_id, search_opts=search_opts,
marker=marker, limit=limit, offset=offset, sort_keys=sort_keys,
sort_direction=sort_dirs)
@wsgi.Controller.api_version(API_VERSION)
@wsgi.response(202)

16
cinder/db/sqlalchemy/api.py
View File

@ -1778,9 +1778,9 @@ def _volume_get(context, volume_id, session=None, joined_load=True):
def _attachment_get_all(context, filters=None, marker=None, limit=None,
offset=None, sort_keys=None, sort_dirs=None):
project_id = filters.pop('project_id', None) if filters else None
if filters and not is_valid_model_filters(models.VolumeAttachment,
filters):
filters,
exclude_list=['project_id']):
return []
session = get_session()
@ -1791,11 +1791,6 @@ def _attachment_get_all(context, filters=None, marker=None, limit=None,
offset, models.VolumeAttachment)
if query is None:
return []
query = query.options(joinedload('volume'))
if project_id:
query = query.filter(models.Volume.project_id == project_id)
return query.all()
@ -1820,9 +1815,16 @@ def _attachment_get_query(context, session=None, project_only=False):
def _process_attachment_filters(query, filters):
if filters:
project_id = filters.pop('project_id', None)
# Ensure that filters' keys exist on the model
if not is_valid_model_filters(models.VolumeAttachment, filters):
return
if project_id:
volume = models.Volume
query = query.filter(volume.id ==
models.VolumeAttachment.volume_id,
volume.project_id == project_id)
query = query.filter_by(**filters)
return query

40
cinder/tests/unit/api/v3/test_attachments.py
View File

@ -42,18 +42,36 @@ class AttachmentsAPITestCase(test.TestCase):
self.ctxt = context.RequestContext(fake.USER_ID, fake.PROJECT_ID,
auth_token=True,
is_admin=True)
self.volume1 = self._create_volume(display_name='fake_volume_1',
project_id=fake.PROJECT_ID)
self.volume2 = self._create_volume(display_name='fake_volume_2',
project_id=fake.PROJECT2_ID)
self.attachment1 = self._create_attachement(
volume_uuid=fake.VOLUME_ID, instance_uuid=fake.UUID1)
volume_uuid=self.volume1.id, instance_uuid=fake.UUID1)
self.attachment2 = self._create_attachement(
volume_uuid=fake.VOLUME2_ID, instance_uuid=fake.UUID1)
volume_uuid=self.volume1.id, instance_uuid=fake.UUID1)
self.attachment3 = self._create_attachement(
volume_uuid=fake.VOLUME3_ID, instance_uuid=fake.UUID2)
volume_uuid=self.volume1.id, instance_uuid=fake.UUID2)
self.attachment4 = self._create_attachement(
volume_uuid=self.volume2.id, instance_uuid=fake.UUID2)
self.addCleanup(self._cleanup)
def _cleanup(self):
self.attachment1.destroy()
self.attachment2.destroy()
self.attachment3.destroy()
self.attachment4.destroy()
self.volume1.destroy()
self.volume2.destroy()
def _create_volume(self, ctxt=None, display_name=None, project_id=None):
"""Create a volume object."""
ctxt = ctxt or self.ctxt
volume = objects.Volume(ctxt)
volume.display_name = display_name
volume.project_id = project_id
volume.create()
return volume
def _create_attachement(self, ctxt=None, volume_uuid=None,
instance_uuid=None, mountpoint=None,
@ -141,3 +159,19 @@ class AttachmentsAPITestCase(test.TestCase):
expect_result = order_ids[2] if sort_dir == "desc" else order_ids[0]
self.assertEqual(expect_result,
res_dict['attachments'][0]['id'])
@ddt.data({'admin': True, 'request_url': '?all_tenants=1', 'count': 4},
{'admin': False, 'request_url': '?all_tenants=1', 'count': 3},
{'admin': True, 'request_url':
'?all_tenants=1&project_id=%s' % fake.PROJECT2_ID,
'count': 1},
{'admin': False, 'request_url': '', 'count': 3})
@ddt.unpack
def test_list_attachment_with_tenants(self, admin, request_url, count):
url = '/v3/%s/attachments%s' % (fake.PROJECT_ID, request_url)
req = fakes.HTTPRequest.blank(url, version=ATTACHMENTS_MICRO_VERSION,
use_admin_context=admin)
res_dict = self.controller.index(req)
self.assertEqual(1, len(res_dict))
self.assertEqual(count, len(res_dict['attachments']))

4
releasenotes/notes/support-tenants-project-in-attachment-list-3edd8g138a28s4r8.yaml Normal file
View File

@ -0,0 +1,4 @@
---
fixes:
- Add ``all_tenants``, ``project_id`` support in
attachment list&detail APIs.