Skip to content

Commit

Permalink
Fix volumes search by metadata
Browse files Browse the repository at this point in the history 
The metadata parameter is urlencoded (to string) by cinderclient
but isn't decoded back to dict by the API.

The metadata parameter needs to be converted back to dict
to be of any use by the filters parameter of get_all().

The original implementation has been done by:
Mathieu Gagne <mgagne@iweb.com>

Fixes: bug #1195015

Change-Id: I19d7d386afddddc067e9f4ef86967c9b72d9e530
  • Loading branch information
@mgagne
mgagne authored and Seif Lotfy committed Jul 21, 2013
1 parent 6e95e1a commit 76fc407
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 2 deletions.
6 changes: 5 additions & 1 deletion cinder/api/v1/volumes.py
View file
Expand Up @@ -15,6 +15,7 @@

"""The volumes api."""

import ast
import webob
from webob import exc

Expand Down Expand Up @@ -253,6 +254,9 @@ def _items(self, req, entity_maker):
search_opts = {}
search_opts.update(req.GET)

if 'metadata' in search_opts:
search_opts['metadata'] = ast.literal_eval(search_opts['metadata'])

context = req.environ['cinder.context']
remove_invalid_options(context,
search_opts, self._get_volume_search_options())
Expand Down Expand Up @@ -362,7 +366,7 @@ def create(self, req, body):

def _get_volume_search_options(self):
"""Return volume search options allowed by non-admin."""
return ('display_name', 'status')
return ('display_name', 'status', 'metadata')

@wsgi.serializers(xml=VolumeTemplate)
def update(self, req, id, body):
Expand Down
6 changes: 5 additions & 1 deletion cinder/api/v2/volumes.py
View file
Expand Up @@ -16,6 +16,7 @@
"""The volumes api."""


import ast
import webob
from webob import exc

Expand Down Expand Up @@ -214,6 +215,9 @@ def _get_volumes(self, req, is_detail):
filters['display_name'] = filters['name']
del filters['name']

if 'metadata' in filters:
filters['metadata'] = ast.literal_eval(filters['metadata'])

volumes = self.volume_api.get_all(context, marker, limit, sort_key,
sort_dir, filters)
limited_list = common.limited(volumes, req)
Expand Down Expand Up @@ -322,7 +326,7 @@ def create(self, req, body):

def _get_volume_filter_options(self):
"""Return volume search options allowed by non-admin."""
return ('name', 'status')
return ('name', 'status', 'metadata')

@wsgi.serializers(xml=VolumeTemplate)
def update(self, req, id, body):
Expand Down
59 changes: 59 additions & 0 deletions cinder/tests/api/v1/test_volumes.py
View file
Expand Up @@ -17,6 +17,7 @@

from lxml import etree
from oslo.config import cfg
import urllib
import webob

from cinder.api import extensions
Expand Down Expand Up @@ -364,6 +365,64 @@ def stub_volume_get_all_by_project(context, project_id, marker, limit,
resp = self.controller.index(req)
self.assertEqual(len(resp['volumes']), 0)

def test_volume_list_by_metadata(self):
def stub_volume_get_all_by_project(context, project_id, marker, limit,
sort_key, sort_dir):
return [
stubs.stub_volume(1, display_name='vol1',
status='available',
volume_metadata=[{'key': 'key1',
'value': 'value1'}]),
stubs.stub_volume(2, display_name='vol2',
status='available',
volume_metadata=[{'key': 'key1',
'value': 'value2'}]),
stubs.stub_volume(3, display_name='vol3',
status='in-use',
volume_metadata=[{'key': 'key1',
'value': 'value2'}]),
]
self.stubs.Set(db, 'volume_get_all_by_project',
stub_volume_get_all_by_project)

# no metadata filter
req = fakes.HTTPRequest.blank('/v1/volumes', use_admin_context=True)
resp = self.controller.index(req)
self.assertEqual(len(resp['volumes']), 3)

# single match
qparams = urllib.urlencode({'metadata': {'key1': 'value1'}})
req = fakes.HTTPRequest.blank('/v1/volumes?%s' % qparams,
use_admin_context=True)
resp = self.controller.index(req)
self.assertEqual(len(resp['volumes']), 1)
self.assertEqual(resp['volumes'][0]['display_name'], 'vol1')
self.assertEqual(resp['volumes'][0]['metadata']['key1'], 'value1')

# multiple matches
qparams = urllib.urlencode({'metadata': {'key1': 'value2'}})
req = fakes.HTTPRequest.blank('/v1/volumes?%s' % qparams,
use_admin_context=True)
resp = self.controller.index(req)
self.assertEqual(len(resp['volumes']), 2)
for volume in resp['volumes']:
self.assertEqual(volume['metadata']['key1'], 'value2')

# multiple filters
qparams = urllib.urlencode({'metadata': {'key1': 'value2'}})
req = fakes.HTTPRequest.blank('/v1/volumes?status=in-use&%s' % qparams,
use_admin_context=True)
resp = self.controller.index(req)
self.assertEqual(len(resp['volumes']), 1)
self.assertEqual(resp['volumes'][0]['display_name'], 'vol3')

# no match
qparams = urllib.urlencode({'metadata': {'key1': 'value3'}})
req = fakes.HTTPRequest.blank('/v1/volumes?%s' % qparams,
use_admin_context=True)
resp = self.controller.index(req)
self.assertEqual(len(resp['volumes']), 0)

def test_volume_list_by_status(self):
def stub_volume_get_all_by_project(context, project_id, marker, limit,
sort_key, sort_dir):
Expand Down
59 changes: 59 additions & 0 deletions cinder/tests/api/v2/test_volumes.py
View file
Expand Up @@ -18,6 +18,7 @@

from lxml import etree
from oslo.config import cfg
import urllib
import webob

from cinder.api import extensions
Expand Down Expand Up @@ -555,6 +556,64 @@ def stub_volume_get_all_by_project(context, project_id, marker, limit,
resp = self.controller.index(req)
self.assertEqual(len(resp['volumes']), 0)

def test_volume_list_by_metadata(self):
def stub_volume_get_all_by_project(context, project_id, marker, limit,
sort_key, sort_dir):
return [
stubs.stub_volume(1, display_name='vol1',
status='available',
volume_metadata=[{'key': 'key1',
'value': 'value1'}]),
stubs.stub_volume(2, display_name='vol2',
status='available',
volume_metadata=[{'key': 'key1',
'value': 'value2'}]),
stubs.stub_volume(3, display_name='vol3',
status='in-use',
volume_metadata=[{'key': 'key1',
'value': 'value2'}]),
]
self.stubs.Set(db, 'volume_get_all_by_project',
stub_volume_get_all_by_project)

# no metadata filter
req = fakes.HTTPRequest.blank('/v2/volumes', use_admin_context=True)
resp = self.controller.detail(req)
self.assertEqual(len(resp['volumes']), 3)

# single match
qparams = urllib.urlencode({'metadata': {'key1': 'value1'}})
req = fakes.HTTPRequest.blank('/v2/volumes?%s' % qparams,
use_admin_context=True)
resp = self.controller.detail(req)
self.assertEqual(len(resp['volumes']), 1)
self.assertEqual(resp['volumes'][0]['name'], 'vol1')
self.assertEqual(resp['volumes'][0]['metadata']['key1'], 'value1')

# multiple matches
qparams = urllib.urlencode({'metadata': {'key1': 'value2'}})
req = fakes.HTTPRequest.blank('/v2/volumes?%s' % qparams,
use_admin_context=True)
resp = self.controller.detail(req)
self.assertEqual(len(resp['volumes']), 2)
for volume in resp['volumes']:
self.assertEqual(volume['metadata']['key1'], 'value2')

# multiple filters
qparams = urllib.urlencode({'metadata': {'key1': 'value2'}})
req = fakes.HTTPRequest.blank('/v2/volumes?status=in-use&%s' % qparams,
use_admin_context=True)
resp = self.controller.detail(req)
self.assertEqual(len(resp['volumes']), 1)
self.assertEqual(resp['volumes'][0]['name'], 'vol3')

# no match
qparams = urllib.urlencode({'metadata': {'key1': 'value3'}})
req = fakes.HTTPRequest.blank('/v2/volumes?%s' % qparams,
use_admin_context=True)
resp = self.controller.detail(req)
self.assertEqual(len(resp['volumes']), 0)

def test_volume_list_by_status(self):
def stub_volume_get_all_by_project(context, project_id, marker, limit,
sort_key, sort_dir):
Expand Down

0 comments on commit 76fc407

Please sign in to comment.