Add the ability to update type public status

Currently, the v2 volume type update api doesn't support updating
volume type's public status. All volume types created are public
by default if not specified. It is not possible to update an existing
volume_type is_public status. It is necessary to add updating public
status for volume type. If a volume type updated from public to
private, the volumes previously created with this type will not be
affected, but the user without access will not be able to create
volume with this type anymore.

APIImpact: Add 'is_public' for v2 type update api.
Implements blueprint: volume-types-public-update

Change-Id: I4a5b2cf8f747c87908a09835a5edb8873c4e946b

diff --git a/cinder/api/contrib/types_manage.py b/cinder/api/contrib/types_manage.py
index 9c32528d7972b39bade862f64fb42fcf35f2678d..56afb9317b28c21d3af99ea043e03b8bf54a4a03 100644 (file)
--- a/cinder/api/contrib/types_manage.py
+++ b/cinder/api/contrib/types_manage.py
@@ -112,6 +112,7 @@ class VolumeTypesManageController(wsgi.Controller):
         vol_type = body['volume_type']
         description = vol_type.get('description')
         name = vol_type.get('name')
+        is_public = vol_type.get('is_public')
 
         # Name and description can not be both None.
         # If name specified, name can not be empty.
@@ -123,6 +124,11 @@ class VolumeTypesManageController(wsgi.Controller):
             msg = _("Specify either volume type name and/or description.")
             raise webob.exc.HTTPBadRequest(explanation=msg)
 
+        if is_public is not None and not utils.is_valid_boolstr(is_public):
+            msg = _("Invalid value '%s' for is_public. Accepted values: "
+                    "True or False.") % is_public
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+
         if name:
             utils.check_string_length(name, 'Type name',
                                       min_length=1, max_length=255)
@@ -132,7 +138,8 @@ class VolumeTypesManageController(wsgi.Controller):
                                       min_length=0, max_length=255)
 
         try:
-            volume_types.update(context, id, name, description)
+            volume_types.update(context, id, name, description,
+                                is_public=is_public)
             # Get the updated
             vol_type = volume_types.get_volume_type(context, id)
             req.cache_resource(vol_type, name='types')

diff --git a/cinder/api/views/types.py b/cinder/api/views/types.py
index 78058df0a3df7f443d6844f1079cd6ae3d508e51..c8c1d4ff9570b626f845638b94b538df96734b60 100644 (file)
--- a/cinder/api/views/types.py
+++ b/cinder/api/views/types.py
@@ -22,6 +22,7 @@ class ViewBuilder(common.ViewBuilder):
         """Trim away extraneous volume type attributes."""
         trimmed = dict(id=volume_type.get('id'),
                        name=volume_type.get('name'),
+                       is_public=volume_type.get('is_public'),
                        extra_specs=volume_type.get('extra_specs'),
                        description=volume_type.get('description'))
         return trimmed if brief else dict(volume_type=trimmed)

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index fd1bfb54f2ee508fdced7551faeadaec8e9bb4e8..ec96b1725c9fda4250427b57213b28df138fa630 100644 (file)
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -2302,6 +2302,10 @@ def volume_type_update(context, volume_type_id, values):
         if values['description'] is None:
             del values['description']
 
+        # No is_public change
+        if values['is_public'] is None:
+            del values['is_public']
+
         # No name change
         if values['name'] is None:
             del values['name']

diff --git a/cinder/tests/unit/api/contrib/test_types_manage.py b/cinder/tests/unit/api/contrib/test_types_manage.py
index f613f7487da3aeee6a5e0879559bc056ec71b9b5..542206ef2ca38e14707f0a8ef71b38ad5e836c2b 100644 (file)
--- a/cinder/tests/unit/api/contrib/test_types_manage.py
+++ b/cinder/tests/unit/api/contrib/test_types_manage.py
@@ -36,9 +36,10 @@ def stub_volume_type(id):
                 extra_specs=specs)
 
 
-def stub_volume_type_updated(id):
+def stub_volume_type_updated(id, is_public=True):
     return dict(id=id,
                 name='vol_type_%s_%s' % (six.text_type(id), six.text_type(id)),
+                is_public=is_public,
                 description='vol_type_desc_%s_%s' % (
                     six.text_type(id), six.text_type(id)))
 
@@ -84,14 +85,6 @@ def return_volume_types_create_duplicate_type(context,
     raise exception.VolumeTypeExists(id=name)
 
 
-def return_volume_types_update(context, id, name, description):
-    pass
-
-
-def return_volume_types_update_fail(context, id, name, description):
-    raise exception.VolumeTypeUpdateFailed(id=id)
-
-
 def stub_volume_type_updated_name_only(id):
     return dict(id=id,
                 name='vol_type_%s_%s' % (six.text_type(id), six.text_type(id)),
@@ -104,11 +97,7 @@ def stub_volume_type_updated_name_after_delete(id):
                 description='vol_type_desc_%s' % six.text_type(id))
 
 
-def return_volume_types_update_exist(context, id, name, description):
-    raise exception.VolumeTypeExists(id=id, name=name)
-
-
-def return_volume_types_get_volume_type_updated(context, id):
+def return_volume_types_get_volume_type_updated(id, is_public=True):
     if id == "777":
         raise exception.VolumeTypeNotFound(volume_type_id=id)
     if id == '888':
@@ -119,7 +108,7 @@ def return_volume_types_get_volume_type_updated(context, id):
         return stub_volume_type_updated_name_after_delete(int(id))
 
     # anything else
-    return stub_volume_type_updated(int(id))
+    return stub_volume_type_updated(int(id), is_public=is_public)
 
 
 def return_volume_types_get_by_name(context, name):
@@ -273,14 +262,15 @@ class VolumeTypesManageApiTest(test.TestCase):
         body = {'volume_type': 'string'}
         self._create_volume_type_bad_body(body=body)
 
-    def test_update(self):
-        self.stubs.Set(volume_types, 'update',
-                       return_volume_types_update)
-        self.stubs.Set(volume_types, 'get_volume_type',
-                       return_volume_types_get_volume_type_updated)
+    @mock.patch('cinder.volume.volume_types.update')
+    @mock.patch('cinder.volume.volume_types.get_volume_type')
+    def test_update(self, mock_get, mock_update):
+        mock_get.return_value = return_volume_types_get_volume_type_updated(
+            '1', is_public=False)
 
         body = {"volume_type": {"name": "vol_type_1_1",
-                                "description": "vol_type_desc_1_1"}}
+                                "description": "vol_type_desc_1_1",
+                                "is_public": False}}
         req = fakes.HTTPRequest.blank('/v2/fake/types/1')
         req.method = 'PUT'
 
@@ -289,7 +279,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         self.assertEqual(1, len(self.notifier.notifications))
         self._check_test_results(res_dict,
                                  {'expected_desc': 'vol_type_desc_1_1',
-                                  'expected_name': 'vol_type_1_1'})
+                                  'expected_name': 'vol_type_1_1',
+                                  'is_public': False})
 
     @mock.patch('cinder.volume.volume_types.update')
     @mock.patch('cinder.volume.volume_types.get_volume_type')
@@ -325,11 +316,11 @@ class VolumeTypesManageApiTest(test.TestCase):
         self.assertRaises(exception.InvalidInput,
                           self.controller._update, req, '1', body)
 
-    def test_update_non_exist(self):
-        self.stubs.Set(volume_types, 'update',
-                       return_volume_types_update)
-        self.stubs.Set(volume_types, 'get_volume_type',
-                       return_volume_types_get_volume_type)
+    @mock.patch('cinder.volume.volume_types.get_volume_type')
+    @mock.patch('cinder.volume.volume_types.update')
+    def test_update_non_exist(self, mock_update, mock_get_volume_type):
+        mock_get_volume_type.side_effect = exception.VolumeTypeNotFound(
+            volume_type_id=777)
 
         body = {"volume_type": {"name": "vol_type_1_1",
                                 "description": "vol_type_desc_1_1"}}
@@ -341,11 +332,11 @@ class VolumeTypesManageApiTest(test.TestCase):
                           self.controller._update, req, '777', body)
         self.assertEqual(1, len(self.notifier.notifications))
 
-    def test_update_db_fail(self):
-        self.stubs.Set(volume_types, 'update',
-                       return_volume_types_update_fail)
-        self.stubs.Set(volume_types, 'get_volume_type',
-                       return_volume_types_get_volume_type)
+    @mock.patch('cinder.volume.volume_types.get_volume_type')
+    @mock.patch('cinder.volume.volume_types.update')
+    def test_update_db_fail(self, mock_update, mock_get_volume_type):
+        mock_update.side_effect = exception.VolumeTypeUpdateFailed(id='1')
+        mock_get_volume_type.return_value = stub_volume_type(1)
 
         body = {"volume_type": {"name": "vol_type_1_1",
                                 "description": "vol_type_desc_1_1"}}
@@ -374,11 +365,11 @@ class VolumeTypesManageApiTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest,
                           self.controller._update, req, '1', body)
 
-    def test_update_only_name(self):
-        self.stubs.Set(volume_types, 'update',
-                       return_volume_types_update)
-        self.stubs.Set(volume_types, 'get_volume_type',
-                       return_volume_types_get_volume_type_updated)
+    @mock.patch('cinder.volume.volume_types.update')
+    @mock.patch('cinder.volume.volume_types.get_volume_type')
+    def test_update_only_name(self, mock_get, mock_update):
+        mock_get.return_value = return_volume_types_get_volume_type_updated(
+            '999')
 
         body = {"volume_type": {"name": "vol_type_999_999"}}
         req = fakes.HTTPRequest.blank('/v2/fake/types/999')
@@ -391,11 +382,11 @@ class VolumeTypesManageApiTest(test.TestCase):
                                  {'expected_name': 'vol_type_999_999',
                                   'expected_desc': 'vol_type_desc_999'})
 
-    def test_update_only_description(self):
-        self.stubs.Set(volume_types, 'update',
-                       return_volume_types_update)
-        self.stubs.Set(volume_types, 'get_volume_type',
-                       return_volume_types_get_volume_type_updated)
+    @mock.patch('cinder.volume.volume_types.update')
+    @mock.patch('cinder.volume.volume_types.get_volume_type')
+    def test_update_only_description(self, mock_get, mock_update):
+        mock_get.return_value = return_volume_types_get_volume_type_updated(
+            '888')
 
         body = {"volume_type": {"description": "vol_type_desc_888_888"}}
         req = fakes.HTTPRequest.blank('/v2/fake/types/888')
@@ -408,11 +399,23 @@ class VolumeTypesManageApiTest(test.TestCase):
                                  {'expected_name': 'vol_type_888',
                                   'expected_desc': 'vol_type_desc_888_888'})
 
-    def test_rename_existing_name(self):
-        self.stubs.Set(volume_types, 'update',
-                       return_volume_types_update_exist)
-        self.stubs.Set(volume_types, 'get_volume_type',
-                       return_volume_types_get_volume_type_updated)
+    def test_update_invalid_is_public(self):
+        body = {"volume_type": {"name": "test",
+                                "description": "something",
+                                "is_public": "fake"}}
+        req = fakes.HTTPRequest.blank('/v2/fake/types/1')
+        req.method = 'PUT'
+
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller._update, req, '1', body)
+
+    @mock.patch('cinder.volume.volume_types.update')
+    @mock.patch('cinder.volume.volume_types.get_volume_type')
+    def test_rename_existing_name(self, mock_get, mock_update):
+        mock_update.side_effect = exception.VolumeTypeExists(
+            id="666", name="vol_type_666")
+        mock_get.return_value = return_volume_types_get_volume_type_updated(
+            '666')
         # first attempt fail
         body = {"volume_type": {"name": "vol_type_666"}}
         req = fakes.HTTPRequest.blank('/v2/fake/types/666')
@@ -434,10 +437,7 @@ class VolumeTypesManageApiTest(test.TestCase):
         self.assertEqual(1, len(self.notifier.notifications))
 
         # update again
-        self.stubs.Set(volume_types, 'update',
-                       return_volume_types_update)
-        self.stubs.Set(volume_types, 'get_volume_type',
-                       return_volume_types_get_volume_type_updated)
+        mock_update.side_effect = mock.MagicMock()
         body = {"volume_type": {"name": "vol_type_666_666"}}
         req = fakes.HTTPRequest.blank('/v2/fake/types/666')
         req.method = 'PUT'
@@ -457,3 +457,6 @@ class VolumeTypesManageApiTest(test.TestCase):
         if expected_results.get('expected_name'):
             self.assertEqual(expected_results['expected_name'],
                              results['volume_type']['name'])
+        if expected_results.get('is_public') is not None:
+            self.assertEqual(expected_results['is_public'],
+                             results['volume_type']['is_public'])

diff --git a/cinder/tests/unit/api/v1/test_types.py b/cinder/tests/unit/api/v1/test_types.py
index 86c791569351695753cf0abb8eaf21fedf8c9855..cdb9b4e8348afa4b396dabee20b309cf8d431944 100644 (file)
--- a/cinder/tests/unit/api/v1/test_types.py
+++ b/cinder/tests/unit/api/v1/test_types.py
@@ -128,6 +128,7 @@ class VolumeTypesApiTest(test.TestCase):
         expected_volume_type = dict(name='new_type',
                                     extra_specs={},
                                     description=None,
+                                    is_public=None,
                                     id=42)
         self.assertDictMatch(output['volume_type'], expected_volume_type)
 
@@ -154,6 +155,7 @@ class VolumeTypesApiTest(test.TestCase):
             expected_volume_type = dict(name='new_type',
                                         extra_specs={},
                                         id=42 + i,
+                                        is_public=None,
                                         description=None)
             self.assertDictMatch(output['volume_types'][i],
                                  expected_volume_type)

diff --git a/cinder/tests/unit/api/v2/test_types.py b/cinder/tests/unit/api/v2/test_types.py
index 9fc86169e490b82258d37c4b9a6851205203b4fe..f819cbdcdc6a5b83e19d5e76ffdb1017f8cfe32d 100644 (file)
--- a/cinder/tests/unit/api/v2/test_types.py
+++ b/cinder/tests/unit/api/v2/test_types.py
@@ -153,6 +153,7 @@ class VolumeTypesApiTest(test.TestCase):
         raw_volume_type = dict(
             name='new_type',
             description='new_type_desc',
+            is_public=True,
             deleted=False,
             created_at=now,
             updated_at=now,
@@ -168,6 +169,7 @@ class VolumeTypesApiTest(test.TestCase):
         expected_volume_type = dict(
             name='new_type',
             description='new_type_desc',
+            is_public=True,
             extra_specs={},
             id=42,
         )
@@ -183,6 +185,7 @@ class VolumeTypesApiTest(test.TestCase):
                 dict(
                     name='new_type',
                     description='new_type_desc',
+                    is_public=True,
                     deleted=False,
                     created_at=now,
                     updated_at=now,
@@ -200,6 +203,7 @@ class VolumeTypesApiTest(test.TestCase):
             expected_volume_type = dict(
                 name='new_type',
                 description='new_type_desc',
+                is_public=True,
                 extra_specs={},
                 id=42 + i
             )

diff --git a/cinder/volume/volume_types.py b/cinder/volume/volume_types.py
index 8aa9b2916b0f6e80724e53eb7a02639f6c38e498..a6fc171412ffbb5e9b000881bbdaf959e79a93fc 100644 (file)
--- a/cinder/volume/volume_types.py
+++ b/cinder/volume/volume_types.py
@@ -57,7 +57,7 @@ def create(context,
     return type_ref
 
 
-def update(context, id, name, description):
+def update(context, id, name, description, is_public=None):
     """Update volume type by id."""
     if id is None:
         msg = _("id cannot be None")
@@ -66,7 +66,8 @@ def update(context, id, name, description):
         type_updated = db.volume_type_update(context,
                                              id,
                                              dict(name=name,
-                                                  description=description))
+                                                  description=description,
+                                                  is_public=is_public))
     except db_exc.DBError:
         LOG.exception(_LE('DB error:'))
         raise exception.VolumeTypeUpdateFailed(id=id)