Add support of new extra spec 'snapshot_support'

On last midcycle meetup was decided to make snapshots optional feature.
So, add following features:

- Allow to setup 'snapshot_support' extra spec in share type creation
  request. Make it optional key with default 'True' value.
- Display this extra spec with 'type list' command as part of 'optional'
  extra specs.

Change-Id: Id8e11b591a713654f7237cb1b66aeeaebfbf5871
Implements blueprint snapshots-optional

manilaclient/tests/functional/base.py

@@ -144,12 +144,14 @@ class BaseTestCase(base.ClientTestBase):
 
     @classmethod
     def create_share_type(cls, name=None, driver_handles_share_servers=True,
+                          snapshot_support=True,
                           is_public=True, client=None, cleanup_in_class=True):
         if client is None:
             client = cls.get_admin_client()
         share_type = client.create_share_type(
             name=name,
             driver_handles_share_servers=driver_handles_share_servers,
+            snapshot_support=snapshot_support,
             is_public=is_public)
         resource = {
             "type": "share_type",

manilaclient/tests/functional/client.py

@@ -129,13 +129,15 @@ class ManilaCLIClient(base.CLIClient):
     # Share types
 
     def create_share_type(self, name=None, driver_handles_share_servers=True,
-                          is_public=True):
+                          snapshot_support=True, is_public=True):
         """Creates share type.
 
         :param name: text -- name of share type to use, if not set then
             autogenerated will be used
         :param driver_handles_share_servers: bool/str -- boolean or its
             string alias. Default is True.
+        :param snapshot_support: bool/str -- boolean or its
+            string alias. Default is True.
         :param is_public: bool/str -- boolean or its string alias. Default is
             True.
         """
@@ -144,10 +146,14 @@ class ManilaCLIClient(base.CLIClient):
         dhss = driver_handles_share_servers
         if not isinstance(dhss, six.string_types):
             dhss = six.text_type(dhss)
+        if not isinstance(snapshot_support, six.string_types):
+            snapshot_support = six.text_type(snapshot_support)
         if not isinstance(is_public, six.string_types):
             is_public = six.text_type(is_public)
-        cmd = 'type-create %(name)s %(dhss)s --is-public %(is_public)s' % {
+        cmd = ('type-create %(name)s %(dhss)s --is-public %(is_public)s '
-            'name': name, 'dhss': dhss, 'is_public': is_public}
+               '--snapshot-support %(snapshot_support)s') % {
+                   'name': name, 'dhss': dhss, 'is_public': is_public,
+                   'snapshot_support': snapshot_support}
         share_type_raw = self.manila(cmd)
 
         # NOTE(vponomaryov): share type creation response is "list"-like with

manilaclient/tests/functional/test_share_types.py

@@ -69,16 +69,21 @@ class ShareTypesReadWriteTest(base.BaseTestCase):
         {'is_public': True, 'dhss': True},
         {'is_public': False, 'dhss': True},
         {'is_public': False, 'dhss': False},
+        {'is_public': False, 'dhss': True, 'snapshot_support': False},
+        {'is_public': False, 'dhss': False, 'snapshot_support': True},
     )
     @ddt.unpack
-    def test_create_delete_share_type(self, is_public, dhss):
+    def test_create_delete_share_type(self, is_public, dhss,
+                                      snapshot_support=True):
         share_type_name = data_utils.rand_name('manilaclient_functional_test')
         dhss_expected = 'driver_handles_share_servers : %s' % dhss
+        snapshot_support_expected = 'snapshot_support : %s' % snapshot_support
 
         # Create share type
         share_type = self.create_share_type(
             name=share_type_name,
             driver_handles_share_servers=dhss,
+            snapshot_support=snapshot_support,
             is_public=is_public)
 
         st_id = share_type['ID']
@@ -88,6 +93,8 @@ class ShareTypesReadWriteTest(base.BaseTestCase):
             self.assertIn(key, share_type)
         self.assertEqual(share_type_name, share_type['Name'])
         self.assertEqual(dhss_expected, share_type['required_extra_specs'])
+        self.assertEqual(
+            snapshot_support_expected, share_type['optional_extra_specs'])
         self.assertEqual('public' if is_public else 'private',
                          share_type['Visibility'].lower())
         self.assertEqual('-', share_type['is_default'])

manilaclient/tests/unit/v1/test_shell.py

@@ -272,7 +272,8 @@ class ShellTest(test_utils.TestCase):
         self.assert_called('GET', '/types')
         cliutils.print_list.assert_called_once_with(
             mock.ANY,
-            ['ID', 'Name', 'Visibility', 'is_default', 'required_extra_specs'],
+            ['ID', 'Name', 'Visibility', 'is_default', 'required_extra_specs',
+             'optional_extra_specs'],
             mock.ANY)
 
     def test_type_list_default_volume_type(self):
@@ -288,7 +289,10 @@ class ShellTest(test_utils.TestCase):
         expected = {
             'share_type': {
                 'name': 'test-type-3',
-                'extra_specs': {'driver_handles_share_servers': False},
+                'extra_specs': {
+                    'driver_handles_share_servers': False,
+                    'snapshot_support': True,
+                },
                 'os-share-type-access:is_public': public
             }
         }
@@ -498,11 +502,11 @@ class ShellTest(test_utils.TestCase):
             mock.ANY, ['ID', 'Name', 'all_extra_specs'], mock.ANY)
 
     @ddt.data('fake', 'FFFalse', 'trueee')
-    def test_type_create_invalid_extra_spec(self, extra_spec):
+    def test_type_create_invalid_dhss_value(self, value):
         self.assertRaises(
             exceptions.CommandError,
             self.run_command,
-            'type-create test ' + extra_spec,
+            'type-create test ' + value,
         )
 
     @ddt.unpack
@@ -516,7 +520,8 @@ class ShellTest(test_utils.TestCase):
                 "name": "test",
                 "os-share-type-access:is_public": True,
                 "extra_specs": {
-                    "driver_handles_share_servers": expected_bool
+                    "driver_handles_share_servers": expected_bool,
+                    "snapshot_support": True,
                 }
             }
         }
@@ -525,6 +530,37 @@ class ShellTest(test_utils.TestCase):
 
         self.assert_called('POST', '/types', body=expected)
 
+    @ddt.unpack
+    @ddt.data(
+        *([{'expected_bool': True, 'text': v}
+           for v in ('true', 'True', '1', 'TRUE', 'tRuE')] +
+          [{'expected_bool': False, 'text': v}
+           for v in ('false', 'False', '0', 'FALSE', 'fAlSe')])
+    )
+    def test_create_with_snapshot_support(self, expected_bool, text):
+        expected = {
+            "share_type": {
+                "name": "test",
+                "os-share-type-access:is_public": True,
+                "extra_specs": {
+                    "driver_handles_share_servers": False,
+                    "snapshot_support": expected_bool,
+                }
+            }
+        }
+
+        self.run_command('type-create test false --snapshot-support ' + text)
+
+        self.assert_called('POST', '/types', body=expected)
+
+    @ddt.data('fake', 'FFFalse', 'trueee')
+    def test_type_create_invalid_snapshot_support_value(self, value):
+        self.assertRaises(
+            exceptions.CommandError,
+            self.run_command,
+            'type-create test false --snapshot-support ' + value,
+        )
+
     @ddt.data('--is-public', '--is_public')
     def test_update(self, alias):
         # basic rename with positional agruments

manilaclient/tests/unit/v1/test_types.py

@@ -23,33 +23,82 @@ cs = fakes.FakeClient()
 
 @ddt.ddt
 class TypesTest(utils.TestCase):
+
+    @ddt.data(
+        {'snapshot_support': 'False'},
+        {'snapshot_support': 'False', 'foo': 'bar'},
+    )
+    def test_init(self, extra_specs):
+        info = {'extra_specs': {'snapshot_support': 'False'}}
+
+        share_type = share_types.ShareType(share_types.ShareTypeManager, info)
+
+        self.assertTrue(hasattr(share_type, '_required_extra_specs'))
+        self.assertTrue(hasattr(share_type, '_optional_extra_specs'))
+        self.assertTrue(isinstance(share_type._required_extra_specs, dict))
+        self.assertTrue(isinstance(share_type._optional_extra_specs, dict))
+        self.assertEqual(
+            {'snapshot_support': 'False'}, share_type.get_optional_keys())
+
     def test_list_types(self):
         tl = cs.share_types.list()
         cs.assert_called('GET', '/types?is_public=all')
         for t in tl:
             self.assertIsInstance(t, share_types.ShareType)
             self.assertTrue(callable(getattr(t, 'get_required_keys', '')))
+            self.assertTrue(callable(getattr(t, 'get_optional_keys', '')))
             self.assertEqual({'test': 'test'}, t.get_required_keys())
+            self.assertEqual(
+                {'snapshot_support': 'unknown'}, t.get_optional_keys())
 
     def test_list_types_only_public(self):
         cs.share_types.list(show_all=False)
         cs.assert_called('GET', '/types')
 
-    @ddt.data(True, False)
+    @ddt.data(
-    def test_create(self, is_public):
+        (True, True, True),
+        (True, True, False),
+        (True, False, True),
+        (False, True, True),
+        (True, False, False),
+        (False, False, True),
+        (False, True, False),
+        (False, False, False),
+    )
+    @ddt.unpack
+    def test_create(self, is_public, dhss, snapshot_support):
         expected_body = {
             "share_type": {
                 "name": 'test-type-3',
                 'os-share-type-access:is_public': is_public,
                 "extra_specs": {
-                    "driver_handles_share_servers": True
+                    "driver_handles_share_servers": dhss,
+                    "snapshot_support": snapshot_support,
                 }
             }
         }
 
-        t = cs.share_types.create('test-type-3', True, is_public=is_public)
+        result = cs.share_types.create(
+            'test-type-3', dhss, snapshot_support, is_public=is_public)
         cs.assert_called('POST', '/types', expected_body)
-        self.assertIsInstance(t, share_types.ShareType)
+        self.assertIsInstance(result, share_types.ShareType)
+
+    @ddt.data(True, False)
+    def test_create_with_default_values(self, dhss):
+        expected_body = {
+            "share_type": {
+                "name": 'test-type-3',
+                'os-share-type-access:is_public': True,
+                "extra_specs": {
+                    "driver_handles_share_servers": dhss,
+                    "snapshot_support": True,
+                }
+            }
+        }
+
+        result = cs.share_types.create('test-type-3', dhss)
+        cs.assert_called('POST', '/types', expected_body)
+        self.assertIsInstance(result, share_types.ShareType)
 
     def test_set_key(self):
         t = cs.share_types.get(1)

manilaclient/v1/share_types.py

@@ -28,6 +28,10 @@ class ShareType(common_base.Resource):
     def __init__(self, manager, info, loaded=False):
         super(ShareType, self).__init__(manager, info, loaded)
         self._required_extra_specs = info.get('required_extra_specs', {})
+        self._optional_extra_specs = {
+            'snapshot_support': info.get('extra_specs', {}).get(
+                'snapshot_support', 'unknown'),
+        }
 
     def __repr__(self):
         return "<ShareType: %s>" % self.name
@@ -59,6 +63,9 @@ class ShareType(common_base.Resource):
     def get_required_keys(self):
         return self._required_extra_specs
 
+    def get_optional_keys(self):
+        return self._optional_extra_specs
+
     def set_keys(self, metadata):
         """Set extra specs on a share type.
 
@@ -123,7 +130,8 @@ class ShareTypeManager(base.ManagerWithFind):
         """
         self._delete("/types/%s" % common_base.getid(share_type))
 
-    def create(self, name, spec_driver_handles_share_servers, is_public=True):
+    def create(self, name, spec_driver_handles_share_servers,
+               spec_snapshot_support=True, is_public=True):
         """Create a share type.
 
         :param name: Descriptive name of the share type
@@ -136,7 +144,8 @@ class ShareTypeManager(base.ManagerWithFind):
                 "os-share-type-access:is_public": is_public,
                 "extra_specs": {
                     "driver_handles_share_servers":
-                        spec_driver_handles_share_servers
+                        spec_driver_handles_share_servers,
+                    "snapshot_support": spec_snapshot_support,
                 },
             }
         }

manilaclient/v1/shell.py

@@ -1769,6 +1769,13 @@ def _print_type_required_extra_specs(share_type):
         return "N/A"
 
 
+def _print_type_optional_extra_specs(share_type):
+    try:
+        return _print_dict(share_type.get_optional_keys())
+    except exceptions.NotFound:
+        return "N/A"
+
+
 def _print_share_type_list(stypes, default_share_type=None):
 
     def _is_default(share_type):
@@ -1784,13 +1791,21 @@ def _print_share_type_list(stypes, default_share_type=None):
         'Visibility': is_public,
         'is_default': _is_default,
         'required_extra_specs': _print_type_required_extra_specs,
+        'optional_extra_specs': _print_type_optional_extra_specs,
     }
 
     for stype in stypes:
         stype = stype.to_dict()
         stype['Visibility'] = stype.pop('is_public', 'unknown')
 
-    fields = ['ID', 'Name', 'Visibility', 'is_default', 'required_extra_specs']
+    fields = [
+        'ID',
+        'Name',
+        'Visibility',
+        'is_default',
+        'required_extra_specs',
+        'optional_extra_specs',
+    ]
     cliutils.print_list(stypes, fields, formatters)
 
 
@@ -1842,6 +1857,13 @@ def do_extra_specs_list(cs, args):
     type=str,
     help="Required extra specification. "
          "Valid values are 'true'/'1' and 'false'/'0'")
+@cliutils.arg(
+    '--snapshot_support',
+    '--snapshot-support',
+    metavar='<snapshot_support>',
+    action='single_alias',
+    help="Boolean extra spec that used for filtering of back ends by their "
+         "capability to create share snapshots. (Default is True).")
 @cliutils.arg(
     '--is_public',
     '--is-public',
@@ -1851,17 +1873,28 @@ def do_extra_specs_list(cs, args):
 @cliutils.service_type('share')
 def do_type_create(cs, args):
     """Create a new share type."""
-
+    kwargs = {
+        "name": args.name,
+        "is_public": strutils.bool_from_string(args.is_public, default=True),
+    }
     try:
-        extra_spec = strutils.bool_from_string(
+        kwargs['spec_driver_handles_share_servers'] = (
-            args.spec_driver_handles_share_servers, strict=True)
+            strutils.bool_from_string(
+                args.spec_driver_handles_share_servers, strict=True))
     except ValueError as e:
         msg = ("Argument spec_driver_handles_share_servers "
                "argument is not valid: %s" % six.text_type(e))
         raise exceptions.CommandError(msg)
+    try:
+        if args.snapshot_support:
+            kwargs['spec_snapshot_support'] = strutils.bool_from_string(
+                args.snapshot_support, strict=True)
+    except ValueError as e:
+        msg = ("Argument 'snapshot_support' is of boolean type and has "
+               "invalid value: %s" % six.text_type(e))
+        raise exceptions.CommandError(msg)
 
-    is_public = strutils.bool_from_string(args.is_public, default=True)
+    stype = cs.share_types.create(**kwargs)
-    stype = cs.share_types.create(args.name, extra_spec, is_public=is_public)
     _print_share_type_list([stype])