Add update share-type to SDK and CLI

Currently, only the name and description and public access of share-type
is set when the share-type is created, and not allowed to be edited
after the share-type is created. We can only set extra spec for share-type.
But not name or description or public access for share-type.

Change-Id: Ia0df0d46e11d0438e16fc910fc377f9dd4e85521
Partially-Implements: blueprint update-share-type-name-or-description
Depends-On: https://review.opendev.org/669651

manilaclient/api_versions.py

@@ -27,7 +27,7 @@ from manilaclient import utils
 
 LOG = logging.getLogger(__name__)
 
-MAX_VERSION = '2.49'
+MAX_VERSION = '2.50'
 MIN_VERSION = '2.0'
 DEPRECATED_VERSION = '1.0'
 _VERSIONED_METHOD_MAP = {}

manilaclient/tests/functional/base.py

@@ -211,6 +211,25 @@ class BaseTestCase(base.ClientTestBase):
             cls.method_resources.insert(0, resource)
         return share_type
 
+    @classmethod
+    def update_share_type(cls, share_type_id, name=None,
+                          is_public=None, client=None,
+                          microversion=None, description=None):
+        if client is None:
+            client = cls.get_admin_client()
+        data = {
+            "share_type_id": share_type_id,
+            "microversion": microversion,
+        }
+        if name is not None:
+            data["name"] = name
+        if description is not None:
+            data["description"] = description
+        if is_public is not None:
+            data["is_public"] = is_public
+        share_type = client.update_share_type(**data)
+        return share_type
+
     @classmethod
     def create_share_network(cls, name=None, description=None,
                              neutron_net_id=None,

manilaclient/tests/functional/client.py

@@ -244,6 +244,41 @@ class ManilaCLIClient(base.CLIClient):
         share_type = utils.details(share_type_raw)
         return share_type
 
+    def update_share_type(self, share_type_id, name=None,
+                          is_public=None, microversion=None,
+                          description=None):
+        """Update share type.
+
+        :param share_type_id: text -- id of share type.
+        :param name: text -- new name of share type, if not set then
+            it will not be updated.
+        :param description: text -- new description of share type.
+            if not set then it will not be updated.
+        :param is_public: bool/str -- boolean or its string alias.
+            new visibility of the share type.If set to True, share
+            type will be available to all projects in the cloud.
+        """
+
+        cmd = ('type-update %(share_type_id)s ') % {
+            'share_type_id': share_type_id}
+
+        if is_public is not None:
+            if not isinstance(is_public, six.string_types):
+                is_public = six.text_type(is_public)
+            cmd += " --is_public " + is_public
+
+        if description:
+            cmd += " --description " + description
+        elif description == "":
+            cmd += ' --description "" '
+
+        if name:
+            cmd += " --name " + name
+
+        share_type_raw = self.manila(cmd, microversion=microversion)
+        share_type = utils.details(share_type_raw)
+        return share_type
+
     @not_found_wrapper
     def delete_share_type(self, share_type, microversion=None):
         """Deletes share type by its Name or ID."""

manilaclient/tests/functional/test_share_types.py

@@ -122,6 +122,108 @@ class ShareTypesReadWriteTest(base.BaseTestCase):
             '2.41', True, False, None, None, None, None, None,
             description=data_utils.rand_name('test_share_type_description'))
 
+    @ddt.data(
+        ('name_updated_1', 'description_updated', True),
+        ('name_updated_2', 'description_updated', False),
+        ('name_updated_3', None, None),
+        (None, 'description_updated', None),
+        (None, None, True),
+        (None, None, False),
+    )
+    @ddt.unpack
+    def test_create_update_delete_share_type_2_50(self, new_name,
+                                                  new_description,
+                                                  new_is_public):
+        self.skip_if_microversion_not_supported('2.50')
+        microversion = '2.50'
+        share_type_name = data_utils.rand_name('share_type_update_test')
+
+        # Create share type
+        share_type = self.create_share_type(
+            name=share_type_name,
+            driver_handles_share_servers=False,
+            snapshot_support=None,
+            create_share_from_snapshot=None,
+            revert_to_snapshot=None,
+            mount_snapshot=None,
+            is_public=True,
+            microversion=microversion,
+            extra_specs={},
+            description="share_type_description")
+
+        st_id = share_type['ID']
+
+        # Update share type
+        st_updated = self.update_share_type(st_id, name=new_name,
+                                            description=new_description,
+                                            is_public=new_is_public,
+                                            microversion=microversion)
+        # Verify type name
+        if new_name:
+            self.assertEqual(new_name, st_updated['Name'])
+
+        # Verify type description
+        if new_description:
+            self.assertEqual(new_description, st_updated['Description'])
+
+        # Verify public
+        if new_is_public is not None:
+            self.assertEqual('public' if new_is_public else 'private',
+                             st_updated['Visibility'].lower())
+
+        # Delete share type
+        self.admin_client.delete_share_type(st_id, microversion=microversion)
+
+        # Wait for share type deletion
+        self.admin_client.wait_for_share_type_deletion(
+            st_id, microversion=microversion)
+
+        # Verify that it is not listed with common 'type-list' operation.
+        share_types = self.admin_client.list_share_types(
+            list_all=False, microversion=microversion)
+        self.assertFalse(any(st_id == st['ID'] for st in share_types))
+
+    def test_unset_share_type_description_2_50(self):
+        self.skip_if_microversion_not_supported('2.50')
+        microversion = '2.50'
+        share_type_name = data_utils.rand_name('share_type_update_test')
+
+        # Create share type
+        share_type = self.create_share_type(
+            name=share_type_name,
+            driver_handles_share_servers=False,
+            snapshot_support=None,
+            create_share_from_snapshot=None,
+            revert_to_snapshot=None,
+            mount_snapshot=None,
+            is_public=True,
+            microversion=microversion,
+            extra_specs={},
+            description="share_type_description")
+
+        st_id = share_type['ID']
+
+        # Update share type
+        new_description = ""
+        st_updated = self.update_share_type(st_id,
+                                            description=new_description,
+                                            microversion=microversion)
+
+        # Verify type description
+        self.assertEqual('None', st_updated['Description'])
+
+        # Delete share type
+        self.admin_client.delete_share_type(st_id, microversion=microversion)
+
+        # Wait for share type deletion
+        self.admin_client.wait_for_share_type_deletion(
+            st_id, microversion=microversion)
+
+        # Verify that it is not listed with common 'type-list' operation.
+        share_types = self.admin_client.list_share_types(
+            list_all=False, microversion=microversion)
+        self.assertFalse(any(st_id == st['ID'] for st in share_types))
+
     def _test_create_delete_share_type(self, microversion, is_public, dhss,
                                        spec_snapshot_support,
                                        spec_create_share_from_snapshot,

manilaclient/tests/unit/v2/fakes.py

@@ -925,8 +925,9 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
 
     def get_types_1234(self, **kw):
         return (200, {}, {
-            'share_type': {'id': 1,
+            'share_type': {'id': 1234,
-                           'name': 'test-type-1',
+                           'name': 'test-type-1234',
+                           'share_type_access:is_public': True,
                            'description': "test share type desc",
                            'extra_specs': {'test': 'test'},
                            'required_extra_specs': {'test': 'test'}}})

manilaclient/tests/unit/v2/test_types.py

@@ -17,6 +17,7 @@ import itertools
 import mock
 
 from manilaclient import api_versions
+from manilaclient import config
 from manilaclient import exceptions
 from manilaclient.tests.unit import utils
 from manilaclient.tests.unit.v2 import fakes
@@ -24,6 +25,10 @@ from manilaclient.v2 import share_types
 
 cs = fakes.FakeClient()
 
+CONF = config.CONF
+
+LATEST_MICROVERSION = CONF.max_api_microversion
+
 
 def get_valid_type_create_data_2_0():
 
@@ -430,6 +435,28 @@ class TypesTest(utils.TestCase):
         t.unset_keys(['k'])
         cs.assert_called('DELETE', '/types/1/extra_specs/k')
 
+    @ddt.data(*set(('2.50', LATEST_MICROVERSION)))
+    def test_update(self, microversion):
+        manager = self._get_share_types_manager(microversion)
+        self.mock_object(manager, '_update', mock.Mock(return_value="fake"))
+        share_type = 1234
+        name = "updated-test-type-1234"
+        description = "updated test description"
+        is_public_key_name = "share_type_access:is_public"
+        is_public = False
+        expected_body = {
+            "share_type": {
+                "name": name,
+                is_public_key_name: is_public,
+            }
+        }
+        result = manager.update(
+            share_type, name, is_public, description)
+        expected_body['share_type']['description'] = description
+        manager._update.assert_called_once_with(
+            "/types/%s" % share_type, expected_body, "share_type")
+        self.assertEqual("fake", result)
+
     def test_delete(self):
         cs.share_types.delete(1)
         cs.assert_called('DELETE', '/types/1')

manilaclient/v2/share_types.py

@@ -101,6 +101,10 @@ class ShareType(common_base.Resource):
             if resp is not None:
                 return resp
 
+    def update(self, **kwargs):
+        """Update this share type."""
+        return self.manager.update(self, **kwargs)
+
 
 class ShareTypeManager(base.ManagerWithFind):
     """Manage :class:`ShareType` resources."""
@@ -164,6 +168,30 @@ class ShareTypeManager(base.ManagerWithFind):
             body["share_type"]["description"] = description
         return self._create("/types", body, "share_type")
 
+    def _do_update(self, share_type, name=None, is_public=None,
+                   is_public_keyname="share_type_access:is_public",
+                   description=None):
+        """Update the name and/or description for a share type.
+
+        :param share_type: the ID of the :class: `ShareType` to update.
+        :param name: Descriptive name of the share type.
+        :param description: Description of the share type.
+        :rtype: :class:`ShareType`
+        """
+
+        body = {
+            "share_type": {}
+        }
+
+        if name:
+            body["share_type"]["name"] = name
+        if is_public is not None:
+            body["share_type"][is_public_keyname] = is_public
+        if description or description == "":
+            body["share_type"]["description"] = description
+        return self._update("/types/%s" % common_base.getid(share_type),
+                            body, "share_type")
+
     @api_versions.wraps("1.0", "2.6")
     def create(self, name, spec_driver_handles_share_servers,
                spec_snapshot_support=None, is_public=True, extra_specs=None):
@@ -221,6 +249,11 @@ class ShareTypeManager(base.ManagerWithFind):
         return self._do_create(name, extra_specs, is_public,
                                description=description)
 
+    @api_versions.wraps("2.50")
+    def update(self, share_type, name=None, is_public=None, description=None):
+        return self._do_update(share_type, name, is_public,
+                               description=description)
+
     def _handle_spec_driver_handles_share_servers(
             self, extra_specs, spec_driver_handles_share_servers):
         """Validation and default for DHSS extra spec."""

manilaclient/v2/shell.py

@@ -4131,6 +4131,55 @@ def do_type_create(cs, args):
     _print_share_type(stype, show_des=show_des)
 
 
+@cliutils.arg(
+    'id',
+    metavar='<id>',
+    help="Name or ID of the share type to update.")
+@cliutils.arg(
+    '--name',
+    metavar='<name>',
+    type=str,
+    help="New name of share type.")
+@cliutils.arg(
+    '--description',
+    metavar='<description>',
+    type=str,
+    default=None,
+    help="New description of share type.")
+@cliutils.arg(
+    '--is-public',
+    '--is_public',
+    metavar='<is_public>',
+    action='single_alias',
+    help="New visibility of the share type. If set to True, share type will "
+         "be available to all projects in the cloud.")
+@api_versions.wraps("2.50")
+def do_type_update(cs, args):
+    """Update share type name, description, and/or visibility. (Admin only)."""
+    name = getattr(args, 'name')
+    description = getattr(args, 'description')
+    is_public = getattr(args, 'is_public')
+    if not name and description is None and is_public is None:
+        msg = ("A description and/or non-empty name and/or boolean is_public "
+               "must be supplied to update the respective attributes of the "
+               "share type.")
+        raise exceptions.CommandError(msg)
+    kwargs = {}
+    kwargs['name'] = name
+    if is_public:
+        try:
+            kwargs['is_public'] = strutils.bool_from_string(is_public,
+                                                            strict=True)
+        except ValueError as e:
+            raise exceptions.CommandError("The value of 'is_public' is"
+                                          " invalid: %s", six.text_type(e))
+
+    kwargs['description'] = description
+    stype = _find_share_type(cs, args.id)
+    stype = stype.update(**kwargs)
+    _print_share_type(stype, show_des=True)
+
+
 @cliutils.arg(
     'id',
     metavar='<id>',

releasenotes/notes/bp-update-share-type-name-or-description-32d98b5a42cd8090.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    The ``name``, ``description`` and/or ``share_type_access:is_public``
+    attributes of share types can be updated with API version ``2.50``
+    and beyond.