Fix volumes search by metadata

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

diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py
index 6a0960758239ce6b839fd94002fc21e99d161932..1f75c7105d1081930fa1b985f69aeb0bdf45ae14 100644 (file)
--- a/cinder/api/v1/volumes.py
+++ b/cinder/api/v1/volumes.py
@@ -15,6 +15,7 @@
 
 """The volumes api."""
 
+import ast
 import webob
 from webob import exc
 
@@ -253,6 +254,9 @@ class VolumeController(wsgi.Controller):
         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())
@@ -362,7 +366,7 @@ class VolumeController(wsgi.Controller):
 
     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):

diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py
index b711c70b1fc5d5736d8e6cd0fe09343df74ef786..36e354dfb496b0fd623481478a4885b15d17316f 100644 (file)
--- a/cinder/api/v2/volumes.py
+++ b/cinder/api/v2/volumes.py
@@ -16,6 +16,7 @@
 """The volumes api."""
 
 
+import ast
 import webob
 from webob import exc
 
@@ -214,6 +215,9 @@ class VolumeController(wsgi.Controller):
             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)
@@ -322,7 +326,7 @@ class VolumeController(wsgi.Controller):
 
     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):

diff --git a/cinder/tests/api/v1/test_volumes.py b/cinder/tests/api/v1/test_volumes.py
index 03915852e45bb62718250e98071b6a516f9c584a..c64a762e4a46fa6d37cd3d31d47dc03f600f63d4 100644 (file)
--- a/cinder/tests/api/v1/test_volumes.py
+++ b/cinder/tests/api/v1/test_volumes.py
@@ -17,6 +17,7 @@ import datetime
 
 from lxml import etree
 from oslo.config import cfg
+import urllib
 import webob
 
 from cinder.api import extensions
@@ -364,6 +365,64 @@ class VolumeApiTest(test.TestCase):
         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):

diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py
index e2dbc17616f1195a432c6fcccb34baae1b1c5e8a..db2e09f219b3af36e40a0a40561d23fe69d19198 100644 (file)
--- a/cinder/tests/api/v2/test_volumes.py
+++ b/cinder/tests/api/v2/test_volumes.py
@@ -18,6 +18,7 @@ import datetime
 
 from lxml import etree
 from oslo.config import cfg
+import urllib
 import webob
 
 from cinder.api import extensions
@@ -555,6 +556,64 @@ class VolumeApiTest(test.TestCase):
         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):