Turn db model object into a primitive object to avoid error

In volume API controller we need turn db model object into a primitive
object to avoid error when it adding visible administrator metadata to
volume's metadata dict.

Fixes bug: 1220232

Change-Id: I5f6c4fc8dd6ca3af02db577b37dcaeee92bb42e5
Signed-off-by: Zhi Yan Liu <zhiyanl@cn.ibm.com>

diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py
index df92892b07e736c705e1fb23bd53589d35f52ed8..1cef310585aa8a89f7cd45a331db9d84df342f66 100644 (file)
--- a/cinder/api/v1/volumes.py
+++ b/cinder/api/v1/volumes.py
@@ -222,8 +222,14 @@ class VolumeController(wsgi.Controller):
 
         visible_admin_meta = {}
 
-        volume_tmp = (volume if context.is_admin else
-                      self.volume_api.get(context.elevated(), volume['id']))
+        if context.is_admin:
+            volume_tmp = volume
+        else:
+            try:
+                volume_tmp = self.volume_api.get(context.elevated(),
+                                                 volume['id'])
+            except Exception:
+                return
 
         if volume_tmp.get('volume_admin_metadata'):
             for item in volume_tmp['volume_admin_metadata']:
@@ -242,12 +248,13 @@ class VolumeController(wsgi.Controller):
         # NOTE(zhiyan): update visible administration metadata to
         # volume metadata, administration metadata will rewrite existing key.
         if volume.get('volume_metadata'):
-            orig_meta = volume.get('volume_metadata')
+            orig_meta = list(volume.get('volume_metadata'))
             for item in orig_meta:
                 if item['key'] in visible_admin_meta.keys():
                     item['value'] = visible_admin_meta.pop(item['key'])
             for key, value in visible_admin_meta.iteritems():
                 orig_meta.append({'key': key, 'value': value})
+            volume['volume_metadata'] = orig_meta
         # avoid circular ref when vol is a Volume instance
         elif (volume.get('metadata') and
                 isinstance(volume.get('metadata'), dict)):
@@ -311,6 +318,8 @@ class VolumeController(wsgi.Controller):
                                           sort_key='created_at',
                                           sort_dir='desc', filters=search_opts)
 
+        volumes = [dict(vol.iteritems()) for vol in volumes]
+
         for volume in volumes:
             self._add_visible_admin_metadata(context, volume)
 

diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py
index 29b8d59bb343e5f283a7a677dd35ed792551134f..182edf2517bfba53f96fe58d478f948979dc556d 100644 (file)
--- a/cinder/api/v2/volumes.py
+++ b/cinder/api/v2/volumes.py
@@ -165,8 +165,14 @@ class VolumeController(wsgi.Controller):
 
         visible_admin_meta = {}
 
-        volume_tmp = (volume if context.is_admin else
-                      self.volume_api.get(context.elevated(), volume['id']))
+        if context.is_admin:
+            volume_tmp = volume
+        else:
+            try:
+                volume_tmp = self.volume_api.get(context.elevated(),
+                                                 volume['id'])
+            except Exception:
+                return
 
         if volume_tmp.get('volume_admin_metadata'):
             for item in volume_tmp['volume_admin_metadata']:
@@ -185,12 +191,13 @@ class VolumeController(wsgi.Controller):
         # NOTE(zhiyan): update visible administration metadata to
         # volume metadata, administration metadata will rewrite existing key.
         if volume.get('volume_metadata'):
-            orig_meta = volume.get('volume_metadata')
+            orig_meta = list(volume.get('volume_metadata'))
             for item in orig_meta:
                 if item['key'] in visible_admin_meta.keys():
                     item['value'] = visible_admin_meta.pop(item['key'])
             for key, value in visible_admin_meta.iteritems():
                 orig_meta.append({'key': key, 'value': value})
+            volume['volume_metadata'] = orig_meta
         # avoid circular ref when vol is a Volume instance
         elif (volume.get('metadata') and
                 isinstance(volume.get('metadata'), dict)):
@@ -267,6 +274,8 @@ class VolumeController(wsgi.Controller):
         volumes = self.volume_api.get_all(context, marker, limit, sort_key,
                                           sort_dir, filters)
 
+        volumes = [dict(vol.iteritems()) for vol in volumes]
+
         for volume in volumes:
             self._add_visible_admin_metadata(context, volume)
 

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 34f4699bc82c4a4eebe41766b710df7c79e72532..8de30757c06938636965943aff2ef5a2816aa73b 100644 (file)
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -1046,6 +1046,8 @@ def volume_create(context, values):
         values['volume_admin_metadata'] = \
             _metadata_refs(values.get('admin_metadata'),
                            models.VolumeAdminMetadata)
+    elif values.get('volume_admin_metadata'):
+        del values['volume_admin_metadata']
 
     volume_ref = models.Volume()
     if not values.get('id'):

diff --git a/cinder/tests/api/v1/test_volumes.py b/cinder/tests/api/v1/test_volumes.py
index 48fda37be98e66c650c723f3460a24608cb16a54..ee8fa197051e4f3a1bfdf6e17a8d762d8a6ef1a8 100644 (file)
--- a/cinder/tests/api/v1/test_volumes.py
+++ b/cinder/tests/api/v1/test_volumes.py
@@ -61,15 +61,12 @@ class VolumeApiTest(test.TestCase):
         self.controller = volumes.VolumeController(self.ext_mgr)
 
         self.stubs.Set(db, 'volume_get_all', stubs.stub_volume_get_all)
-        self.stubs.Set(db, 'volume_get_all_by_project',
-                       stubs.stub_volume_get_all_by_project)
         self.stubs.Set(db, 'service_get_all_by_topic',
                        stubs.stub_service_get_all_by_topic)
-        self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
-        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
         self.stubs.Set(volume_api.API, 'delete', stubs.stub_volume_delete)
 
     def test_volume_create(self):
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
         self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create)
 
         vol = {"size": 100,
@@ -142,6 +139,8 @@ class VolumeApiTest(test.TestCase):
                           req, body)
 
     def test_volume_create_with_image_id(self):
+        self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
+
         self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create)
         self.ext_mgr.extensions = {'os-image-create': 'fake'}
         test_id = "c905cedb-7281-47e4-8a62-f26bc5fc4c77"
@@ -206,7 +205,9 @@ class VolumeApiTest(test.TestCase):
                           body)
 
     def test_volume_update(self):
+        self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
         self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update)
+
         updates = {
             "display_name": "Updated Test Name",
         }
@@ -237,7 +238,9 @@ class VolumeApiTest(test.TestCase):
         self.assertEqual(res_dict, expected)
 
     def test_volume_update_metadata(self):
+        self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
         self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update)
+
         updates = {
             "metadata": {"qos_max_iops": 2000}
         }
@@ -269,6 +272,51 @@ class VolumeApiTest(test.TestCase):
         }}
         self.assertEqual(res_dict, expected)
 
+    def test_volume_update_with_admin_metadata(self):
+        self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update)
+
+        volume = stubs.stub_volume("1")
+        del volume['name']
+        del volume['volume_type']
+        del volume['volume_type_id']
+        volume['metadata'] = {'key': 'value'}
+        db.volume_create(context.get_admin_context(), volume)
+        db.volume_admin_metadata_update(context.get_admin_context(), "1",
+                                        {"readonly": "True",
+                                         "invisible_key": "invisible_value"},
+                                        False)
+
+        updates = {
+            "display_name": "Updated Test Name",
+        }
+        body = {"volume": updates}
+        req = fakes.HTTPRequest.blank('/v1/volumes/1')
+        admin_ctx = context.RequestContext('admin', 'fakeproject', True)
+        req.environ['cinder.context'] = admin_ctx
+        res_dict = self.controller.update(req, '1', body)
+        expected = {'volume': {
+            'status': 'fakestatus',
+            'display_description': 'displaydesc',
+            'availability_zone': 'fakeaz',
+            'display_name': 'Updated Test Name',
+            'attachments': [{
+                'id': '1',
+                'volume_id': '1',
+                'server_id': 'fakeuuid',
+                'host_name': None,
+                'device': '/'
+            }],
+            'bootable': False,
+            'volume_type': 'None',
+            'snapshot_id': None,
+            'source_volid': None,
+            'metadata': {'key': 'value',
+                         'readonly': 'True'},
+            'id': '1',
+            'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
+            'size': 1}}
+        self.assertEquals(res_dict, expected)
+
     def test_update_empty_body(self):
         body = {}
         req = fakes.HTTPRequest.blank('/v1/volumes/1')
@@ -295,6 +343,7 @@ class VolumeApiTest(test.TestCase):
                           req, '1', body)
 
     def test_volume_list(self):
+        self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
         self.stubs.Set(volume_api.API, 'get_all',
                        stubs.stub_volume_get_all_by_project)
 
@@ -321,9 +370,48 @@ class VolumeApiTest(test.TestCase):
                                  'size': 1}]}
         self.assertEqual(res_dict, expected)
 
+    def test_volume_list_with_admin_metadata(self):
+        volume = stubs.stub_volume("1")
+        del volume['name']
+        del volume['volume_type']
+        del volume['volume_type_id']
+        volume['metadata'] = {'key': 'value'}
+        db.volume_create(context.get_admin_context(), volume)
+        db.volume_admin_metadata_update(context.get_admin_context(), "1",
+                                        {"readonly": "True",
+                                         "invisible_key": "invisible_value"},
+                                        False)
+
+        req = fakes.HTTPRequest.blank('/v1/volumes')
+        admin_ctx = context.RequestContext('admin', 'fakeproject', True)
+        req.environ['cinder.context'] = admin_ctx
+        res_dict = self.controller.index(req)
+        expected = {'volumes': [{'status': 'fakestatus',
+                                 'display_description': 'displaydesc',
+                                 'availability_zone': 'fakeaz',
+                                 'display_name': 'displayname',
+                                 'attachments': [{'device': '/',
+                                                  'server_id': 'fakeuuid',
+                                                  'host_name': None,
+                                                  'id': '1',
+                                                  'volume_id': '1'}],
+                                 'bootable': False,
+                                 'volume_type': 'None',
+                                 'snapshot_id': None,
+                                 'source_volid': None,
+                                 'metadata': {'key': 'value',
+                                              'readonly': 'True'},
+                                 'id': '1',
+                                 'created_at': datetime.datetime(1, 1, 1,
+                                                                 1, 1, 1),
+                                 'size': 1}]}
+        self.assertEqual(res_dict, expected)
+
     def test_volume_list_detail(self):
+        self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
         self.stubs.Set(volume_api.API, 'get_all',
                        stubs.stub_volume_get_all_by_project)
+
         req = fakes.HTTPRequest.blank('/v1/volumes/detail')
         res_dict = self.controller.index(req)
         expected = {'volumes': [{'status': 'fakestatus',
@@ -347,6 +435,43 @@ class VolumeApiTest(test.TestCase):
                                  'size': 1}]}
         self.assertEqual(res_dict, expected)
 
+    def test_volume_list_detail_with_admin_metadata(self):
+        volume = stubs.stub_volume("1")
+        del volume['name']
+        del volume['volume_type']
+        del volume['volume_type_id']
+        volume['metadata'] = {'key': 'value'}
+        db.volume_create(context.get_admin_context(), volume)
+        db.volume_admin_metadata_update(context.get_admin_context(), "1",
+                                        {"readonly": "True",
+                                         "invisible_key": "invisible_value"},
+                                        False)
+
+        req = fakes.HTTPRequest.blank('/v1/volumes/detail')
+        admin_ctx = context.RequestContext('admin', 'fakeproject', True)
+        req.environ['cinder.context'] = admin_ctx
+        res_dict = self.controller.index(req)
+        expected = {'volumes': [{'status': 'fakestatus',
+                                 'display_description': 'displaydesc',
+                                 'availability_zone': 'fakeaz',
+                                 'display_name': 'displayname',
+                                 'attachments': [{'device': '/',
+                                                  'server_id': 'fakeuuid',
+                                                  'host_name': None,
+                                                  'id': '1',
+                                                  'volume_id': '1'}],
+                                 'bootable': False,
+                                 'volume_type': 'None',
+                                 'snapshot_id': None,
+                                 'source_volid': None,
+                                 'metadata': {'key': 'value',
+                                              'readonly': 'True'},
+                                 'id': '1',
+                                 'created_at': datetime.datetime(1, 1, 1,
+                                                                 1, 1, 1),
+                                 'size': 1}]}
+        self.assertEqual(res_dict, expected)
+
     def test_volume_list_by_name(self):
         def stub_volume_get_all_by_project(context, project_id, marker, limit,
                                            sort_key, sort_dir):
@@ -355,6 +480,7 @@ class VolumeApiTest(test.TestCase):
                 stubs.stub_volume(2, display_name='vol2'),
                 stubs.stub_volume(3, display_name='vol3'),
             ]
+        self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
         self.stubs.Set(db, 'volume_get_all_by_project',
                        stub_volume_get_all_by_project)
 
@@ -438,8 +564,10 @@ class VolumeApiTest(test.TestCase):
                 stubs.stub_volume(2, display_name='vol2', status='available'),
                 stubs.stub_volume(3, display_name='vol3', status='in-use'),
             ]
+        self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
         self.stubs.Set(db, 'volume_get_all_by_project',
                        stub_volume_get_all_by_project)
+
         # no status filter
         req = fakes.HTTPRequest.blank('/v1/volumes')
         resp = self.controller.index(req)
@@ -469,6 +597,8 @@ class VolumeApiTest(test.TestCase):
         self.assertEqual(len(resp['volumes']), 0)
 
     def test_volume_show(self):
+        self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
+
         req = fakes.HTTPRequest.blank('/v1/volumes/1')
         res_dict = self.controller.show(req, '1')
         expected = {'volume': {'status': 'fakestatus',
@@ -566,6 +696,8 @@ class VolumeApiTest(test.TestCase):
 
             self.stubs.Set(db, 'volume_get_all_by_project',
                            stub_volume_get_all_by_project)
+            self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
+
             req = fakes.HTTPRequest.blank('/v1/volumes/detail?limit=2\
                                           &offset=1',
                                           use_admin_context=is_admin)
@@ -579,7 +711,46 @@ class VolumeApiTest(test.TestCase):
         #non_admin case
         volume_detail_limit_offset(is_admin=False)
 
+    def test_volume_show_with_admin_metadata(self):
+        volume = stubs.stub_volume("1")
+        del volume['name']
+        del volume['volume_type']
+        del volume['volume_type_id']
+        volume['metadata'] = {'key': 'value'}
+        db.volume_create(context.get_admin_context(), volume)
+        db.volume_admin_metadata_update(context.get_admin_context(), "1",
+                                        {"readonly": "True",
+                                         "invisible_key": "invisible_value"},
+                                        False)
+
+        req = fakes.HTTPRequest.blank('/v1/volumes/1')
+        admin_ctx = context.RequestContext('admin', 'fakeproject', True)
+        req.environ['cinder.context'] = admin_ctx
+        res_dict = self.controller.show(req, '1')
+        expected = {'volume': {'status': 'fakestatus',
+                               'display_description': 'displaydesc',
+                               'availability_zone': 'fakeaz',
+                               'display_name': 'displayname',
+                               'attachments': [{'device': '/',
+                                                'server_id': 'fakeuuid',
+                                                'host_name': None,
+                                                'id': '1',
+                                                'volume_id': '1'}],
+                               'bootable': False,
+                               'volume_type': 'None',
+                               'snapshot_id': None,
+                               'source_volid': None,
+                               'metadata': {'key': 'value',
+                                            'readonly': 'True'},
+                               'id': '1',
+                               'created_at': datetime.datetime(1, 1, 1,
+                                                               1, 1, 1),
+                               'size': 1}}
+        self.assertEqual(res_dict, expected)
+
     def test_volume_delete(self):
+        self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
+
         req = fakes.HTTPRequest.blank('/v1/volumes/1')
         resp = self.controller.delete(req, 1)
         self.assertEqual(resp.status_int, 202)
@@ -594,6 +765,9 @@ class VolumeApiTest(test.TestCase):
                           1)
 
     def test_admin_list_volumes_limited_to_project(self):
+        self.stubs.Set(db, 'volume_get_all_by_project',
+                       stubs.stub_volume_get_all_by_project)
+
         req = fakes.HTTPRequest.blank('/v1/fake/volumes',
                                       use_admin_context=True)
         res = self.controller.index(req)
@@ -609,17 +783,54 @@ class VolumeApiTest(test.TestCase):
         self.assertEqual(3, len(res['volumes']))
 
     def test_all_tenants_non_admin_gets_all_tenants(self):
+        self.stubs.Set(db, 'volume_get_all_by_project',
+                       stubs.stub_volume_get_all_by_project)
+        self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
+
         req = fakes.HTTPRequest.blank('/v1/fake/volumes?all_tenants=1')
         res = self.controller.index(req)
         self.assertIn('volumes', res)
         self.assertEqual(1, len(res['volumes']))
 
     def test_non_admin_get_by_project(self):
+        self.stubs.Set(db, 'volume_get_all_by_project',
+                       stubs.stub_volume_get_all_by_project)
+        self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db)
+
         req = fakes.HTTPRequest.blank('/v1/fake/volumes')
         res = self.controller.index(req)
         self.assertIn('volumes', res)
         self.assertEqual(1, len(res['volumes']))
 
+    def test_add_visible_admin_metadata_visible_key_only(self):
+        admin_metadata = [{"key": "invisible_key", "value": "invisible_value"},
+                          {"key": "readonly", "value": "visible"},
+                          {"key": "attached_mode", "value": "visible"}]
+        metadata = [{"key": "key", "value": "value"}]
+        volume = dict(volume_admin_metadata=admin_metadata,
+                      volume_metadata=metadata)
+        admin_ctx = context.get_admin_context()
+        self.controller._add_visible_admin_metadata(admin_ctx,
+                                                    volume)
+        self.assertEqual(volume['volume_metadata'],
+                         [{"key": "key", "value": "value"},
+                          {"key": "readonly", "value": "visible"},
+                          {"key": "attached_mode", "value": "visible"}])
+
+        admin_metadata = {"invisible_key": "invisible_value",
+                          "readonly": "visible",
+                          "attached_mode": "visible"}
+        metadata = {"key": "value"}
+        volume = dict(admin_metadata=admin_metadata,
+                      metadata=metadata)
+        admin_ctx = context.get_admin_context()
+        self.controller._add_visible_admin_metadata(admin_ctx,
+                                                    volume)
+        self.assertEqual(volume['metadata'],
+                         {'key': 'value',
+                          'attached_mode': 'visible',
+                          'readonly': 'visible'})
+
 
 class VolumeSerializerTest(test.TestCase):
     def _verify_volume_attachment(self, attach, tree):

diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py
index 0f02b794ee8e86a3836535475042ab1915d69543..d4f1aeab250ced15d4f5987c16baa08fd80a0a3b 100644 (file)
--- a/cinder/tests/api/v2/test_volumes.py
+++ b/cinder/tests/api/v2/test_volumes.py
@@ -64,15 +64,13 @@ class VolumeApiTest(test.TestCase):
         self.controller = volumes.VolumeController(self.ext_mgr)
 
         self.stubs.Set(db, 'volume_get_all', stubs.stub_volume_get_all)
-        self.stubs.Set(db, 'volume_get_all_by_project',
-                       stubs.stub_volume_get_all_by_project)
-        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
         self.stubs.Set(volume_api.API, 'delete', stubs.stub_volume_delete)
         self.stubs.Set(db, 'service_get_all_by_topic',
                        stubs.stub_service_get_all_by_topic)
         self.maxDiff = None
 
     def test_volume_create(self):
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
         self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create)
 
         vol = {
@@ -155,7 +153,9 @@ class VolumeApiTest(test.TestCase):
                           req, body)
 
     def test_volume_create_with_image_id(self):
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
         self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create)
+
         self.ext_mgr.extensions = {'os-image-create': 'fake'}
         vol = {"size": '1',
                "name": "Volume Test Name",
@@ -218,7 +218,9 @@ class VolumeApiTest(test.TestCase):
                           body)
 
     def test_volume_update(self):
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
         self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update)
+
         updates = {
             "name": "Updated Test Name",
         }
@@ -263,7 +265,9 @@ class VolumeApiTest(test.TestCase):
         self.assertEqual(res_dict, expected)
 
     def test_volume_update_metadata(self):
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
         self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update)
+
         updates = {
             "metadata": {"qos_max_iops": 2000}
         }
@@ -305,6 +309,62 @@ class VolumeApiTest(test.TestCase):
         }}
         self.assertEqual(res_dict, expected)
 
+    def test_volume_update_with_admin_metadata(self):
+        self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update)
+
+        volume = stubs.stub_volume("1")
+        del volume['name']
+        del volume['volume_type']
+        del volume['volume_type_id']
+        volume['metadata'] = {'key': 'value'}
+        db.volume_create(context.get_admin_context(), volume)
+        db.volume_admin_metadata_update(context.get_admin_context(), "1",
+                                        {"readonly": "True",
+                                         "invisible_key": "invisible_value"},
+                                        False)
+
+        updates = {
+            "display_name": "Updated Test Name",
+        }
+        body = {"volume": updates}
+        req = fakes.HTTPRequest.blank('/v1/volumes/1')
+        admin_ctx = context.RequestContext('admin', 'fake', True)
+        req.environ['cinder.context'] = admin_ctx
+        res_dict = self.controller.update(req, '1', body)
+        expected = {'volume': {
+            'status': 'fakestatus',
+            'description': 'displaydesc',
+            'availability_zone': 'fakeaz',
+            'name': 'displayname',
+            'attachments': [{
+                'id': '1',
+                'volume_id': '1',
+                'server_id': 'fakeuuid',
+                'host_name': None,
+                'device': '/',
+            }],
+            'user_id': 'fakeuser',
+            'volume_type': None,
+            'snapshot_id': None,
+            'source_volid': None,
+            'metadata': {'key': 'value',
+                         'readonly': 'True'},
+            'id': '1',
+            'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
+            'size': 1,
+            'links': [
+                {
+                    'href': 'http://localhost/v1/fake/volumes/1',
+                    'rel': 'self'
+                },
+                {
+                    'href': 'http://localhost/fake/volumes/1',
+                    'rel': 'bookmark'
+                }
+            ],
+        }}
+        self.assertEquals(res_dict, expected)
+
     def test_update_empty_body(self):
         body = {}
         req = fakes.HTTPRequest.blank('/v2/volumes/1')
@@ -335,6 +395,8 @@ class VolumeApiTest(test.TestCase):
     def test_volume_list_summary(self):
         self.stubs.Set(volume_api.API, 'get_all',
                        stubs.stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/volumes')
         res_dict = self.controller.index(req)
         expected = {
@@ -360,6 +422,8 @@ class VolumeApiTest(test.TestCase):
     def test_volume_list_detail(self):
         self.stubs.Set(volume_api.API, 'get_all',
                        stubs.stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/volumes/detail')
         res_dict = self.controller.detail(req)
         expected = {
@@ -401,6 +465,62 @@ class VolumeApiTest(test.TestCase):
         }
         self.assertEqual(res_dict, expected)
 
+    def test_volume_list_detail_with_admin_metadata(self):
+        volume = stubs.stub_volume("1")
+        del volume['name']
+        del volume['volume_type']
+        del volume['volume_type_id']
+        volume['metadata'] = {'key': 'value'}
+        db.volume_create(context.get_admin_context(), volume)
+        db.volume_admin_metadata_update(context.get_admin_context(), "1",
+                                        {"readonly": "True",
+                                         "invisible_key": "invisible_value"},
+                                        False)
+
+        req = fakes.HTTPRequest.blank('/v2/volumes/detail')
+        admin_ctx = context.RequestContext('admin', 'fakeproject', True)
+        req.environ['cinder.context'] = admin_ctx
+        res_dict = self.controller.detail(req)
+        expected = {
+            'volumes': [
+                {
+                    'status': 'fakestatus',
+                    'description': 'displaydesc',
+                    'availability_zone': 'fakeaz',
+                    'name': 'displayname',
+                    'attachments': [
+                        {
+                            'device': '/',
+                            'server_id': 'fakeuuid',
+                            'host_name': None,
+                            'id': '1',
+                            'volume_id': '1'
+                        }
+                    ],
+                    'user_id': 'fakeuser',
+                    'volume_type': None,
+                    'snapshot_id': None,
+                    'source_volid': None,
+                    'metadata': {'key': 'value', 'readonly': 'True'},
+                    'id': '1',
+                    'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
+                    'size': 1,
+                    'links': [
+                        {
+                            'href': 'http://localhost/v1/fakeproject'
+                                    '/volumes/1',
+                            'rel': 'self'
+                        },
+                        {
+                            'href': 'http://localhost/fakeproject/volumes/1',
+                            'rel': 'bookmark'
+                        }
+                    ],
+                }
+            ]
+        }
+        self.assertEqual(res_dict, expected)
+
     def test_volume_index_with_marker(self):
         def stub_volume_get_all_by_project(context, project_id, marker, limit,
                                            sort_key, sort_dir):
@@ -410,6 +530,8 @@ class VolumeApiTest(test.TestCase):
             ]
         self.stubs.Set(db, 'volume_get_all_by_project',
                        stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/volumes?marker=1')
         res_dict = self.controller.index(req)
         volumes = res_dict['volumes']
@@ -418,6 +540,10 @@ class VolumeApiTest(test.TestCase):
         self.assertEqual(volumes[1]['id'], 2)
 
     def test_volume_index_limit(self):
+        self.stubs.Set(db, 'volume_get_all_by_project',
+                       stubs.stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/volumes?limit=1')
         res_dict = self.controller.index(req)
         volumes = res_dict['volumes']
@@ -436,6 +562,10 @@ class VolumeApiTest(test.TestCase):
                           req)
 
     def test_volume_index_limit_marker(self):
+        self.stubs.Set(db, 'volume_get_all_by_project',
+                       stubs.stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/volumes?marker=1&limit=1')
         res_dict = self.controller.index(req)
         volumes = res_dict['volumes']
@@ -451,6 +581,8 @@ class VolumeApiTest(test.TestCase):
             ]
         self.stubs.Set(db, 'volume_get_all_by_project',
                        stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/volumes?limit=2&offset=1')
         res_dict = self.controller.index(req)
         volumes = res_dict['volumes']
@@ -476,6 +608,8 @@ class VolumeApiTest(test.TestCase):
             ]
         self.stubs.Set(db, 'volume_get_all_by_project',
                        stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/volumes/detail?marker=1')
         res_dict = self.controller.index(req)
         volumes = res_dict['volumes']
@@ -484,6 +618,10 @@ class VolumeApiTest(test.TestCase):
         self.assertEqual(volumes[1]['id'], 2)
 
     def test_volume_detail_limit(self):
+        self.stubs.Set(db, 'volume_get_all_by_project',
+                       stubs.stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/volumes/detail?limit=1')
         res_dict = self.controller.index(req)
         volumes = res_dict['volumes']
@@ -502,6 +640,10 @@ class VolumeApiTest(test.TestCase):
                           req)
 
     def test_volume_detail_limit_marker(self):
+        self.stubs.Set(db, 'volume_get_all_by_project',
+                       stubs.stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/volumes/detail?marker=1&limit=1')
         res_dict = self.controller.index(req)
         volumes = res_dict['volumes']
@@ -517,6 +659,8 @@ class VolumeApiTest(test.TestCase):
             ]
         self.stubs.Set(db, 'volume_get_all_by_project',
                        stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/volumes/detail?limit=2&offset=1')
         res_dict = self.controller.index(req)
         volumes = res_dict['volumes']
@@ -550,6 +694,7 @@ class VolumeApiTest(test.TestCase):
             ]
         self.stubs.Set(db, 'volume_get_all_by_project',
                        stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
 
         # no name filter
         req = fakes.HTTPRequest.blank('/v2/volumes')
@@ -633,6 +778,8 @@ class VolumeApiTest(test.TestCase):
             ]
         self.stubs.Set(db, 'volume_get_all_by_project',
                        stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         # no status filter
         req = fakes.HTTPRequest.blank('/v2/volumes/details')
         resp = self.controller.detail(req)
@@ -662,6 +809,8 @@ class VolumeApiTest(test.TestCase):
         self.assertEqual(len(resp['volumes']), 0)
 
     def test_volume_show(self):
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/volumes/1')
         res_dict = self.controller.show(req, '1')
         expected = {
@@ -746,7 +895,63 @@ class VolumeApiTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPNotFound, self.controller.show,
                           req, 1)
 
+    def test_volume_show_with_admin_metadata(self):
+        volume = stubs.stub_volume("1")
+        del volume['name']
+        del volume['volume_type']
+        del volume['volume_type_id']
+        volume['metadata'] = {'key': 'value'}
+        db.volume_create(context.get_admin_context(), volume)
+        db.volume_admin_metadata_update(context.get_admin_context(), "1",
+                                        {"readonly": "True",
+                                         "invisible_key": "invisible_value"},
+                                        False)
+
+        req = fakes.HTTPRequest.blank('/v2/volumes/1')
+        admin_ctx = context.RequestContext('admin', 'fakeproject', True)
+        req.environ['cinder.context'] = admin_ctx
+        res_dict = self.controller.show(req, '1')
+        expected = {
+            'volume': {
+                'status': 'fakestatus',
+                'description': 'displaydesc',
+                'availability_zone': 'fakeaz',
+                'name': 'displayname',
+                'attachments': [
+                    {
+                        'device': '/',
+                        'server_id': 'fakeuuid',
+                        'host_name': None,
+                        'id': '1',
+                        'volume_id': '1'
+                    }
+                ],
+                'user_id': 'fakeuser',
+                'volume_type': None,
+                'snapshot_id': None,
+                'source_volid': None,
+                'metadata': {'key': 'value',
+                             'readonly': 'True'},
+                'id': '1',
+                'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
+                'size': 1,
+                'links': [
+                    {
+                        'href': 'http://localhost/v1/fakeproject/volumes/1',
+                        'rel': 'self'
+                    },
+                    {
+                        'href': 'http://localhost/fakeproject/volumes/1',
+                        'rel': 'bookmark'
+                    }
+                ],
+            }
+        }
+        self.assertEqual(res_dict, expected)
+
     def test_volume_delete(self):
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/volumes/1')
         resp = self.controller.delete(req, 1)
         self.assertEqual(resp.status_int, 202)
@@ -755,6 +960,7 @@ class VolumeApiTest(test.TestCase):
         def stub_volume_attached(self, context, volume, force=False):
             raise exception.VolumeAttached(volume_id=volume['id'])
         self.stubs.Set(volume_api.API, "delete", stub_volume_attached)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
 
         req = fakes.HTTPRequest.blank('/v2/volumes/1')
         self.assertRaises(webob.exc.HTTPBadRequest,
@@ -769,6 +975,9 @@ class VolumeApiTest(test.TestCase):
                           req, 1)
 
     def test_admin_list_volumes_limited_to_project(self):
+        self.stubs.Set(db, 'volume_get_all_by_project',
+                       stubs.stub_volume_get_all_by_project)
+
         req = fakes.HTTPRequest.blank('/v2/fake/volumes',
                                       use_admin_context=True)
         res = self.controller.index(req)
@@ -777,6 +986,9 @@ class VolumeApiTest(test.TestCase):
         self.assertEqual(1, len(res['volumes']))
 
     def test_admin_list_volumes_all_tenants(self):
+        self.stubs.Set(db, 'volume_get_all_by_project',
+                       stubs.stub_volume_get_all_by_project)
+
         req = fakes.HTTPRequest.blank('/v2/fake/volumes?all_tenants=1',
                                       use_admin_context=True)
         res = self.controller.index(req)
@@ -784,12 +996,20 @@ class VolumeApiTest(test.TestCase):
         self.assertEqual(3, len(res['volumes']))
 
     def test_all_tenants_non_admin_gets_all_tenants(self):
+        self.stubs.Set(db, 'volume_get_all_by_project',
+                       stubs.stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/fake/volumes?all_tenants=1')
         res = self.controller.index(req)
         self.assertIn('volumes', res)
         self.assertEqual(1, len(res['volumes']))
 
     def test_non_admin_get_by_project(self):
+        self.stubs.Set(db, 'volume_get_all_by_project',
+                       stubs.stub_volume_get_all_by_project)
+        self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get)
+
         req = fakes.HTTPRequest.blank('/v2/fake/volumes')
         res = self.controller.index(req)
         self.assertIn('volumes', res)
@@ -813,6 +1033,35 @@ class VolumeApiTest(test.TestCase):
         body = {'volume': 'string'}
         self._create_volume_bad_request(body=body)
 
+    def test_add_visible_admin_metadata_visible_key_only(self):
+        admin_metadata = [{"key": "invisible_key", "value": "invisible_value"},
+                          {"key": "readonly", "value": "visible"},
+                          {"key": "attached_mode", "value": "visible"}]
+        metadata = [{"key": "key", "value": "value"}]
+        volume = dict(volume_admin_metadata=admin_metadata,
+                      volume_metadata=metadata)
+        admin_ctx = context.get_admin_context()
+        self.controller._add_visible_admin_metadata(admin_ctx,
+                                                    volume)
+        self.assertEqual(volume['volume_metadata'],
+                         [{"key": "key", "value": "value"},
+                          {"key": "readonly", "value": "visible"},
+                          {"key": "attached_mode", "value": "visible"}])
+
+        admin_metadata = {"invisible_key": "invisible_value",
+                          "readonly": "visible",
+                          "attached_mode": "visible"}
+        metadata = {"key": "value"}
+        volume = dict(admin_metadata=admin_metadata,
+                      metadata=metadata)
+        admin_ctx = context.get_admin_context()
+        self.controller._add_visible_admin_metadata(admin_ctx,
+                                                    volume)
+        self.assertEqual(volume['metadata'],
+                         {'key': 'value',
+                          'attached_mode': 'visible',
+                          'readonly': 'visible'})
+
 
 class VolumeSerializerTest(test.TestCase):
     def _verify_volume_attachment(self, attach, tree):