Update volume type name for volume type API

This continues the change for updating volume type to
include changing name.

- Updated the following APIs and tests for volume type

* update volume type
PUT http://<openstackhost>:8776/v2/${tenant_id}/types/${vol_type_id}
body
{
    "volume_type": {
      "name": "test_updated",
      "description":"updated_desc"
    }
}

** user can update name
** if update name, name can not be empty spaces.
** if update name, can not change the name to be the same as the one
in another volume type
** name and description can not be both None

Implements: blueprint volume-type-description
Change-Id: Idc08f796932df709ff20c17f6bc8d4753d85ad3c

diff --git a/cinder/api/contrib/types_manage.py b/cinder/api/contrib/types_manage.py
index 2839fd93977367a6429e1347b9ab66f256af7939..434d4fa4300052b3f2c493cf2ff9073676f66324 100644 (file)
--- a/cinder/api/contrib/types_manage.py
+++ b/cinder/api/contrib/types_manage.py
@@ -99,17 +99,22 @@ class VolumeTypesManageController(wsgi.Controller):
             raise webob.exc.HTTPBadRequest()
 
         vol_type = body['volume_type']
-        description = vol_type.get('description', None)
+        description = vol_type.get('description')
+        name = vol_type.get('name')
 
-        if description is None:
-            msg = _("Specify the description to update.")
+        # Name and description can not be both None.
+        # If name specified, name can not be empty.
+        if name and len(name.strip()) == 0:
+            msg = _("Volume type name can not be empty.")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+
+        if name is None and description is None:
+            msg = _("Specify either volume type name and/or description.")
             raise webob.exc.HTTPBadRequest(explanation=msg)
 
         try:
-            # check it exists
-            vol_type = volume_types.get_volume_type(context, id)
-            volume_types.update(context, id, description)
-            # get the updated
+            volume_types.update(context, id, name, description)
+            # Get the updated
             vol_type = volume_types.get_volume_type(context, id)
             req.cache_resource(vol_type, name='types')
             self._notify_volume_type_info(

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 7b5cc1e637650817addc14e4730899b25b7240fb..e3137dc5c3ec41b68fe05f31d416ae5cc5c3d5ee 100644 (file)
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -2028,13 +2028,35 @@ def _volume_type_get_query(context, session=None, read_deleted=None,
 def volume_type_update(context, volume_type_id, values):
     session = get_session()
     with session.begin():
+        # Check it exists
         volume_type_ref = _volume_type_ref_get(context,
                                                volume_type_id,
                                                session)
-
         if not volume_type_ref:
             raise exception.VolumeTypeNotFound(type_id=volume_type_id)
 
+        # No description change
+        if values['description'] is None:
+            del values['description']
+
+        # No name change
+        if values['name'] is None:
+            del values['name']
+        else:
+            # Volume type name is unique. If change to a name that belongs to
+            # a different volume_type , it should be prevented.
+            check_vol_type = None
+            try:
+                check_vol_type = \
+                    _volume_type_get_by_name(context,
+                                             values['name'],
+                                             session=session)
+            except exception.VolumeTypeNotFoundByName:
+                pass
+            else:
+                if check_vol_type.get('id') != volume_type_id:
+                    raise exception.VolumeTypeExists(id=values['name'])
+
         volume_type_ref.update(values)
         volume_type_ref.save(session=session)
         volume_type = volume_type_get(context, volume_type_id)

diff --git a/cinder/tests/api/contrib/test_types_manage.py b/cinder/tests/api/contrib/test_types_manage.py
index 07901289e7ce228640afb307f9b8fe56bf6c0412..6b8530ea5c240c4cc71412d62445a7678f50dd31 100644 (file)
--- a/cinder/tests/api/contrib/test_types_manage.py
+++ b/cinder/tests/api/contrib/test_types_manage.py
@@ -84,19 +84,39 @@ def return_volume_types_create_duplicate_type(context,
     raise exception.VolumeTypeExists(id=name)
 
 
-def return_volume_types_update(context, id, description):
+def return_volume_types_update(context, id, name, description):
     pass
 
 
-def return_volume_types_update_fail(context, id, description):
+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)),
+                description='vol_type_desc_%s' % six.text_type(id))
+
+
+def stub_volume_type_updated_name_after_delete(id):
+    return dict(id=id,
+                name='vol_type_%s' % six.text_type(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):
     if id == "777":
         raise exception.VolumeTypeNotFound(volume_type_id=id)
     if id == '888':
         return stub_volume_type_updated_desc_only(int(id))
+    if id == '999':
+        return stub_volume_type_updated_name_only(int(id))
+    if id == '666':
+        return stub_volume_type_updated_name_after_delete(int(id))
 
     # anything else
     return stub_volume_type_updated(int(id))
@@ -213,7 +233,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         self.stubs.Set(volume_types, 'get_volume_type',
                        return_volume_types_get_volume_type_updated)
 
-        body = {"volume_type": {"description": "vol_type_desc_1_1"}}
+        body = {"volume_type": {"name": "vol_type_1_1",
+                                "description": "vol_type_desc_1_1"}}
         req = fakes.HTTPRequest.blank('/v2/fake/types/1')
         req.method = 'PUT'
 
@@ -221,7 +242,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         res_dict = self.controller._update(req, '1', body)
         self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
         self._check_test_results(res_dict,
-                                 {'expected_desc': 'vol_type_desc_1_1'})
+                                 {'expected_desc': 'vol_type_desc_1_1',
+                                  'expected_name': 'vol_type_1_1'})
 
     def test_update_non_exist(self):
         self.stubs.Set(volume_types, 'update',
@@ -229,7 +251,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         self.stubs.Set(volume_types, 'get_volume_type',
                        return_volume_types_get_volume_type)
 
-        body = {"volume_type": {"description": "vol_type_desc_1_1"}}
+        body = {"volume_type": {"name": "vol_type_1_1",
+                                "description": "vol_type_desc_1_1"}}
         req = fakes.HTTPRequest.blank('/v2/fake/types/777')
         req.method = 'PUT'
 
@@ -244,7 +267,8 @@ class VolumeTypesManageApiTest(test.TestCase):
         self.stubs.Set(volume_types, 'get_volume_type',
                        return_volume_types_get_volume_type)
 
-        body = {"volume_type": {"description": "vol_type_desc_1_1"}}
+        body = {"volume_type": {"name": "vol_type_1_1",
+                                "description": "vol_type_desc_1_1"}}
         req = fakes.HTTPRequest.blank('/v2/fake/types/1')
         req.method = 'PUT'
 
@@ -253,7 +277,7 @@ class VolumeTypesManageApiTest(test.TestCase):
                           self.controller._update, req, '1', body)
         self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
 
-    def test_update_no_description(self):
+    def test_update_no_name_no_description(self):
         body = {"volume_type": {}}
         req = fakes.HTTPRequest.blank('/v2/fake/types/1')
         req.method = 'PUT'
@@ -261,6 +285,91 @@ class VolumeTypesManageApiTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest,
                           self.controller._update, req, '1', body)
 
+    def test_update_empty_name(self):
+        body = {"volume_type": {"name": "  ",
+                                "description": "something"}}
+        req = fakes.HTTPRequest.blank('/v2/fake/types/1')
+        req.method = 'PUT'
+
+        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)
+
+        body = {"volume_type": {"name": "vol_type_999_999"}}
+        req = fakes.HTTPRequest.blank('/v2/fake/types/999')
+        req.method = 'PUT'
+
+        self.assertEqual(0, len(fake_notifier.NOTIFICATIONS))
+        res_dict = self.controller._update(req, '999', body)
+        self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
+        self._check_test_results(res_dict,
+                                 {'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)
+
+        body = {"volume_type": {"description": "vol_type_desc_888_888"}}
+        req = fakes.HTTPRequest.blank('/v2/fake/types/888')
+        req.method = 'PUT'
+
+        self.assertEqual(0, len(fake_notifier.NOTIFICATIONS))
+        res_dict = self.controller._update(req, '888', body)
+        self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
+        self._check_test_results(res_dict,
+                                 {'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)
+        # first attempt fail
+        body = {"volume_type": {"name": "vol_type_666"}}
+        req = fakes.HTTPRequest.blank('/v2/fake/types/666')
+        req.method = 'PUT'
+
+        self.assertEqual(0, len(fake_notifier.NOTIFICATIONS))
+        self.assertRaises(webob.exc.HTTPConflict,
+                          self.controller._update, req, '666', body)
+        self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
+
+        # delete
+        fake_notifier.reset()
+        self.stubs.Set(volume_types, 'destroy',
+                       return_volume_types_destroy)
+
+        req = fakes.HTTPRequest.blank('/v2/fake/types/1')
+        self.assertEqual(0, len(fake_notifier.NOTIFICATIONS))
+        self.controller._delete(req, '1')
+        self.assertEqual(1, len(fake_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)
+        body = {"volume_type": {"name": "vol_type_666_666"}}
+        req = fakes.HTTPRequest.blank('/v2/fake/types/666')
+        req.method = 'PUT'
+
+        fake_notifier.reset()
+        self.assertEqual(0, len(fake_notifier.NOTIFICATIONS))
+        res_dict = self.controller._update(req, '666', body)
+        self._check_test_results(res_dict,
+                                 {'expected_name': 'vol_type_666',
+                                  'expected_desc': 'vol_type_desc_666'})
+        self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
+
     def _check_test_results(self, results, expected_results):
         self.assertEqual(1, len(results))
         self.assertEqual(expected_results['expected_desc'],

diff --git a/cinder/tests/test_volume_types.py b/cinder/tests/test_volume_types.py
index b6abc15a83a40d537e93d321a79d4f9f5256f23a..f308c442cafc24fdca5769431145dc84757835f8 100644 (file)
--- a/cinder/tests/test_volume_types.py
+++ b/cinder/tests/test_volume_types.py
@@ -78,10 +78,13 @@ class VolumeTypeTestCase(test.TestCase):
                          'drive type was not created')
 
         # update
+        new_type_name = self.vol_type1_name + '_updated'
         new_type_desc = self.vol_type1_description + '_updated'
         type_ref_updated = volume_types.update(self.ctxt,
                                                type_ref.id,
+                                               new_type_name,
                                                new_type_desc)
+        self.assertEqual(new_type_name, type_ref_updated['name'])
         self.assertEqual(new_type_desc, type_ref_updated['description'])
 
         # destroy

diff --git a/cinder/volume/volume_types.py b/cinder/volume/volume_types.py
index 8624a10f4d367568de18c44959fe0d1f23f017cc..207186d4c4b49c7cb276bf5fb644c385255cc7bd 100644 (file)
--- a/cinder/volume/volume_types.py
+++ b/cinder/volume/volume_types.py
@@ -58,7 +58,7 @@ def create(context,
     return type_ref
 
 
-def update(context, id, description):
+def update(context, id, name, description):
     """Update volume type by id."""
     if id is None:
         msg = _("id cannot be None")
@@ -66,7 +66,8 @@ def update(context, id, description):
     try:
         type_updated = db.volume_type_update(context,
                                              id,
-                                             dict(description=description))
+                                             dict(name=name,
+                                                  description=description))
     except db_exc.DBError as e:
         LOG.exception(_LE('DB error: %s') % six.text_type(e))
         raise exception.VolumeTypeUpdateFailed(id=id)