From: Mathieu Gagné <mgagne@iweb.com>
Date: Wed, 26 Jun 2013 20:27:43 +0000 (-0400)
Subject: Fix volumes search by metadata
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=76fc407382315a97cc020e8592199609b3a6a8e9;p=openstack-build%2Fcinder-build.git

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 6a0960758..1f75c7105 100644
--- 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 b711c70b1..36e354dfb 100644
--- 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 03915852e..c64a762e4 100644
--- 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 e2dbc1761..db2e09f21 100644
--- 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):