Add create_share_from_snapshot_support extra spec

The snapshot_support extra spec has always meant two things: a driver
can take snapshots and create shares from snapshots. As we add alternate
snapshot semantics, it is likely that some drivers will want to support
snapshots and some of the new semantics while being unable to create new
shares from snapshots.

This work adds support to manila client for the new extra spec in manila
server, create_share_from_snapshot_support.

Depends-On: Ib0ad5fbfdf6297665c208149b08c8d21b3c232be
Implements: blueprint add-create-share-from-snapshot-extra-spec

Change-Id: I07b70f04e6fb2b5797557c4f4796c6883680eff3

manilaclient/api_versions.py

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

manilaclient/tests/functional/base.py

@@ -171,9 +171,10 @@ class BaseTestCase(base.ClientTestBase):
 
     @classmethod
     def create_share_type(cls, name=None, driver_handles_share_servers=True,
-                          snapshot_support=None, is_public=True, client=None,
+                          snapshot_support=None,
-                          cleanup_in_class=True, microversion=None,
+                          create_share_from_snapshot=None, is_public=True,
-                          extra_specs=None):
+                          client=None, cleanup_in_class=True,
+                          microversion=None, extra_specs=None):
         if client is None:
             client = cls.get_admin_client()
         share_type = client.create_share_type(
@@ -183,6 +184,7 @@ class BaseTestCase(base.ClientTestBase):
             is_public=is_public,
             microversion=microversion,
             extra_specs=extra_specs,
+            create_share_from_snapshot=create_share_from_snapshot,
         )
         resource = {
             "type": "share_type",

manilaclient/tests/functional/client.py

@@ -163,7 +163,8 @@ class ManilaCLIClient(base.CLIClient):
     # Share types
 
     def create_share_type(self, name=None, driver_handles_share_servers=True,
-                          snapshot_support=None, is_public=True,
+                          snapshot_support=None,
+                          create_share_from_snapshot=None, is_public=True,
                           microversion=None, extra_specs=None):
         """Creates share type.
 
@@ -172,10 +173,12 @@ class ManilaCLIClient(base.CLIClient):
         :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.
+            string alias. Default is None.
         :param is_public: bool/str -- boolean or its string alias. Default is
             True.
         :param extra_specs: -- dictionary of extra specs Default is None.
+        :param create_share_from_snapshot: -- boolean or its string
+            alias. Default is None.
         """
         if name is None:
             name = data_utils.rand_name('manilaclient_functional_test')
@@ -193,6 +196,13 @@ class ManilaCLIClient(base.CLIClient):
                 snapshot_support = six.text_type(snapshot_support)
                 cmd += " --snapshot-support " + snapshot_support
 
+        if create_share_from_snapshot is not None:
+            if not isinstance(create_share_from_snapshot, six.string_types):
+                create_share_from_snapshot = six.text_type(
+                    create_share_from_snapshot)
+                cmd += (" --create-share-from-snapshot-support " +
+                        create_share_from_snapshot)
+
         if extra_specs is not None:
             extra_spec_str = ''
             for k, v in extra_specs.items():

manilaclient/tests/functional/test_share_types.py

@@ -16,7 +16,9 @@
 import ddt
 from tempest.lib.common.utils import data_utils
 
+from manilaclient import api_versions
 from manilaclient.tests.functional import base
+from manilaclient.tests.unit.v2 import test_types as unit_test_types
 
 
 @ddt.ddt
@@ -80,85 +82,109 @@ class ShareTypesReadWriteTest(base.BaseTestCase):
                 share_type_id=share_type_id, by_admin=True, list_all=False,
                 microversion=microversion))
 
-    @ddt.data(
+    @ddt.data(*unit_test_types.get_valid_type_create_data_2_0())
-        (True, False, None),
-        (True, True, None),
-        (False, True, None),
-        (False, False, None),
-
-        (False, True, None, {'snapshot_support': False,
-                             'replication_type': 'writable'}, "2.6"),
-        (False, False, None, {'snapshot_support': True,
-                              'replication_type': 'readable'}, "2.7"),
-        (False, False, None, {'snapshot_support': False,
-                              'replication_type': 'dr',
-                              'foo': 'bar'}),
-        (False, False, None, {'replication_type': 'writable',
-                              'foo': 'bar',
-                              'foo2': 'abcd'}),
-        (False, True, True, {'foo2': 'abcd'}),
-        (False, True, False, {'foo2': 'abcd'}),
-        (False, True, False, {'capabilities:dedupe': '<is> True'}),
-    )
     @ddt.unpack
-    def test_create_delete_share_type(self, is_public, dhss,
+    def test_create_delete_share_type(
-                                      spec_snapshot_support=None,
+            self, is_public, dhss, spec_snapshot_support, extra_specs):
-                                      extra_specs=None, microversion=None):
+
-        if microversion:
+        self.skip_if_microversion_not_supported('2.0')
-            self.skip_if_microversion_not_supported(microversion)
+        self._test_create_delete_share_type(
+            '2.0', is_public, dhss, spec_snapshot_support,
+            None, extra_specs)
+
+    @ddt.data(*unit_test_types.get_valid_type_create_data_2_24())
+    @ddt.unpack
+    def test_create_delete_share_type_2_24(
+            self, is_public, dhss, spec_snapshot_support,
+            spec_create_share_from_snapshot, extra_specs):
+
+        self.skip_if_microversion_not_supported('2.24')
+        self._test_create_delete_share_type(
+            '2.24', is_public, dhss, spec_snapshot_support,
+            spec_create_share_from_snapshot, extra_specs)
+
+    def _test_create_delete_share_type(self, microversion, is_public, dhss,
+                                       spec_snapshot_support,
+                                       spec_create_share_from_snapshot,
+                                       extra_specs):
 
         share_type_name = data_utils.rand_name('manilaclient_functional_test')
-        dhss_expected = 'driver_handles_share_servers : %s' % dhss
 
         if extra_specs is None:
             extra_specs = {}
 
-        expected_extra_specs = []
-        for key, val in extra_specs.items():
-            expected_extra_specs.append(('{} : {}'.format(key, val)).strip())
-
-        if 'snapshot_support' not in extra_specs:
-            if spec_snapshot_support is None:
-                expected_extra_specs.append(
-                    ('{} : {}'.format('snapshot_support', True)).strip())
-            else:
-                expected_extra_specs.append(
-                    ('{} : {}'.format(
-                        'snapshot_support',
-                        spec_snapshot_support)).strip())
-
         # Create share type
         share_type = self.create_share_type(
             name=share_type_name,
             driver_handles_share_servers=dhss,
             snapshot_support=spec_snapshot_support,
+            create_share_from_snapshot=spec_create_share_from_snapshot,
             is_public=is_public,
             microversion=microversion,
-            extra_specs=extra_specs,
+            extra_specs=extra_specs)
-        )
+
-        st_id = share_type['ID']
-        optional_extra_specs = share_type['optional_extra_specs']
         # Verify response body
         for key in self.create_keys:
             self.assertIn(key, share_type)
 
+        # Verify type name
         self.assertEqual(share_type_name, share_type['Name'])
+
+        # Verify required DHSS extra spec
+        dhss_expected = 'driver_handles_share_servers : %s' % dhss
         self.assertEqual(dhss_expected, share_type['required_extra_specs'])
-        if not isinstance(optional_extra_specs, list):
+
+        # Determine expected extra specs. Note that prior to 2.24,
+        # the standard 'snapshot_support' extra spec was required.
+        expected_extra_specs = []
+        for key, val in extra_specs.items():
+            expected_extra_specs.append(('{} : {}'.format(key, val)).strip())
+
+        if (api_versions.APIVersion(microversion) <
+                api_versions.APIVersion('2.24')):
+            if 'snapshot_support' not in extra_specs:
+                if spec_snapshot_support is None:
+                    expected_extra_specs.append(
+                        ('{} : {}'.format('snapshot_support', True)).strip())
+                else:
+                    expected_extra_specs.append(
+                        ('{} : {}'.format(
+                            'snapshot_support',
+                            spec_snapshot_support)).strip())
+        else:
+            if spec_snapshot_support is not None:
+                expected_extra_specs.append(
+                    ('{} : {}'.format(
+                        'snapshot_support',
+                        spec_snapshot_support)).strip())
+
+        if spec_create_share_from_snapshot is not None:
+            expected_extra_specs.append(
+                ('{} : {}'.format(
+                    'create_share_from_snapshot_support',
+                    spec_create_share_from_snapshot)).strip())
+
+        # Verify optional extra specs
+        optional_extra_specs = share_type['optional_extra_specs']
+        if optional_extra_specs == '':
+            optional_extra_specs = []
+        elif not isinstance(optional_extra_specs, list):
             optional_extra_specs = [optional_extra_specs]
-        self.assertEqual(len(expected_extra_specs),
+
-                         len(optional_extra_specs))
+        self.assertEqual(len(expected_extra_specs), len(optional_extra_specs))
         for e in optional_extra_specs:
             self.assertIn(e.strip(), expected_extra_specs)
 
+        # Verify public & default attributes
         self.assertEqual('public' if is_public else 'private',
                          share_type['Visibility'].lower())
         self.assertEqual('-', share_type['is_default'])
 
         # Verify its access
-        self._verify_access(
+        st_id = share_type['ID']
-            share_type_id=st_id, is_public=is_public,
+        self._verify_access(share_type_id=st_id,
-            microversion=microversion)
+                            is_public=is_public,
+                            microversion=microversion)
 
         # Delete share type
         self.admin_client.delete_share_type(st_id, microversion=microversion)
@@ -170,8 +196,7 @@ class ShareTypesReadWriteTest(base.BaseTestCase):
         # 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(
+        self.assertFalse(any(st_id == st['ID'] for st in share_types))
-            any(st_id == st['ID'] for st in share_types))
 
     @ddt.data("2.6", "2.7")
     def test_add_remove_access_to_private_share_type(self, microversion):

manilaclient/tests/unit/v2/test_shell.py

@@ -16,6 +16,7 @@
 
 import ddt
 import fixtures
+import itertools
 import mock
 from oslo_utils import strutils
 import six
@@ -449,6 +450,7 @@ class ShellTest(test_utils.TestCase):
                 'extra_specs': {
                     'driver_handles_share_servers': False,
                     'snapshot_support': True,
+                    'create_share_from_snapshot_support': True,
                 },
                 'share_type_access:is_public': public
             }
@@ -783,14 +785,17 @@ class ShellTest(test_utils.TestCase):
             ' --extra-specs driver_handles_share_servers=' + value,
         )
 
-    @ddt.data('True', 'False')
+    @ddt.data(*itertools.product(
-    def test_type_create_duplicate_snapshot_support(self, value):
+        ['snapshot_support', 'create_share_from_snapshot_support'],
-        self.assertRaises(
+        ['True', 'False'])
-            exceptions.CommandError,
+    )
-            self.run_command,
+    @ddt.unpack
-            'type-create test True --snapshot-support ' + value +
+    def test_type_create_duplicate_switch_and_extra_spec(self, key, value):
-            ' --extra-specs snapshot_support=' + value,
+
-        )
+        cmd = ('type-create test True --%(key)s %(value)s --extra-specs '
+               '%(key)s=%(value)s' % {'key': key, 'value': value})
+
+        self.assertRaises(exceptions.CommandError, self.run_command, cmd)
 
     def test_type_create_duplicate_extra_spec_key(self):
         self.assertRaises(
@@ -813,6 +818,7 @@ class ShellTest(test_utils.TestCase):
                 "extra_specs": {
                     "driver_handles_share_servers": expected_bool,
                     "snapshot_support": True,
+                    "create_share_from_snapshot_support": True,
                 }
             }
         }
@@ -828,7 +834,7 @@ class ShellTest(test_utils.TestCase):
           [{'expected_bool': False, 'text': v}
            for v in ('false', 'False', '0', 'FALSE', 'fAlSe')])
     )
-    def test_create_with_snapshot_support(self, expected_bool, text):
+    def test_type_create_with_snapshot_support(self, expected_bool, text):
         expected = {
             "share_type": {
                 "name": "test",
@@ -859,6 +865,7 @@ class ShellTest(test_utils.TestCase):
                 "extra_specs": {
                     "driver_handles_share_servers": False,
                     "snapshot_support": expected_bool,
+                    "create_share_from_snapshot_support": True,
                     "replication_type": replication_type,
                 }
             }
@@ -870,20 +877,46 @@ class ShellTest(test_utils.TestCase):
 
         self.assert_called('POST', '/types', body=expected)
 
-    @ddt.data('fake', 'FFFalse', 'trueee')
+    @ddt.unpack
-    def test_type_create_invalid_snapshot_support_value(self, value):
+    @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_type_create_with_create_share_from_snapshot_support(
+            self, expected_bool, text):
+        expected = {
+            "share_type": {
+                "name": "test",
+                "share_type_access:is_public": True,
+                "extra_specs": {
+                    "driver_handles_share_servers": False,
+                    "snapshot_support": True,
+                    "create_share_from_snapshot_support": expected_bool,
+                }
+            }
+        }
+
+        self.run_command('type-create test false --snapshot-support true '
+                         '--create-share-from-snapshot-support ' + text)
+
+        self.assert_called('POST', '/types', body=expected)
+
+    @ddt.data('snapshot_support', 'create_share_from_snapshot_support')
+    def test_type_create_invalid_switch_value(self, value):
         self.assertRaises(
             exceptions.CommandError,
             self.run_command,
-            'type-create test false --snapshot-support ' + value,
+            'type-create test false --%s fake' % value,
         )
 
-    @ddt.data('fake', 'FFFalse', 'trueee')
+    @ddt.data('snapshot_support', 'create_share_from_snapshot_support')
-    def test_type_create_invalid_snapshot_support_value2(self, value):
+    def test_type_create_invalid_extra_spec_value(self, value):
         self.assertRaises(
             exceptions.CommandError,
             self.run_command,
-            'type-create test false --extra-specs snapshot_support=' + value,
+            'type-create test false --extra-specs %s=fake' % value,
         )
 
     @ddt.data('--is-public', '--is_public')

manilaclient/tests/unit/v2/test_types.py

@@ -11,7 +11,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import copy
 import ddt
+import itertools
 import mock
 
 from manilaclient import api_versions
@@ -23,6 +25,53 @@ from manilaclient.v2 import share_types
 cs = fakes.FakeClient()
 
 
+def get_valid_type_create_data_2_0():
+
+    public = [True, False]
+    dhss = [True, False]
+    snapshot = [None, True, False]
+    extra_specs = [None, {'foo': 'bar'}]
+
+    combos = list(itertools.product(public, dhss, snapshot, extra_specs))
+
+    return combos
+
+
+def get_valid_type_create_data_2_24():
+
+    public = [True, False]
+    dhss = [True, False]
+    snapshot = [None]
+    create_from_snapshot = [None]
+    extra_specs = [None, {'replication_type': 'writable', 'foo': 'bar'}]
+
+    snapshot_none_combos = list(itertools.product(public, dhss, snapshot,
+                                                  create_from_snapshot,
+                                                  extra_specs))
+
+    public = [True, False]
+    dhss = [True, False]
+    snapshot = [True]
+    create_from_snapshot = [True, False, None]
+    extra_specs = [None, {'replication_type': 'readable', 'foo': 'bar'}]
+
+    snapshot_true_combos = list(itertools.product(public, dhss, snapshot,
+                                                  create_from_snapshot,
+                                                  extra_specs))
+
+    public = [True, False]
+    dhss = [True, False]
+    snapshot = [False]
+    create_from_snapshot = [False, None]
+    extra_specs = [None, {'replication_type': 'dr', 'foo': 'bar'}]
+
+    snapshot_false_combos = list(itertools.product(public, dhss, snapshot,
+                                                   create_from_snapshot,
+                                                   extra_specs))
+
+    return snapshot_none_combos + snapshot_true_combos + snapshot_false_combos
+
+
 @ddt.ddt
 class TypesTest(utils.TestCase):
 
@@ -60,38 +109,19 @@ class TypesTest(utils.TestCase):
         cs.share_types.list(show_all=False)
         cs.assert_called('GET', '/types')
 
-    @ddt.data(
+    @ddt.data(*get_valid_type_create_data_2_0())
-        (True, True, None, {'snapshot_support': True}),
-        (True, True, None, {'snapshot_support': True,
-                            'replication_type': 'fake_repl_type',
-                            'foo': 'bar'}),
-        (True, True, None, {'snapshot_support': False,
-                            'replication_type': 'fake_repl_type',
-                            'foo': 'abc'}),
-        (True, False, None, {'snapshot_support': True,
-                             'replication_type': 'fake_repl_type'}),
-        (False, True, None, {'snapshot_support': True,
-                             'replication_type': 'fake_repl_type'}),
-        (True, False, None, {'snapshot_support': False,
-                             'replication_type': 'fake_repl_type'}),
-        (False, False, None, {'snapshot_support': True,
-                              'replication_type': 'fake_repl_type'}),
-        (False, True, None, {'snapshot_support': False,
-                             'replication_type': 'fake_repl_type'}),
-        (False, False, None, {'snapshot_support': False,
-                              'replication_type': 'fake_repl_type'}),
-
-        (False, False, None, {'replication_type': 'fake_repl_type'}),
-        (False, False, True, {'replication_type': 'fake_repl_type'}),
-        (False, False, False, {'replication_type': 'fake_repl_type'}),
-
-        (False, False, False, None),
-        (False, False, True, None),
-        (False, False, None, None),
-    )
     @ddt.unpack
-    def test_create(self, is_public, dhss, spec_snapshot_support, extra_specs):
+    def test_create_2_7(self, is_public, dhss, snapshot, extra_specs):
+
+        extra_specs = copy.copy(extra_specs)
+
         manager = self._get_share_types_manager("2.7")
+        self.mock_object(manager, '_create', mock.Mock(return_value="fake"))
+
+        result = manager.create(
+            'test-type-3', spec_driver_handles_share_servers=dhss,
+            spec_snapshot_support=snapshot, extra_specs=extra_specs,
+            is_public=is_public)
 
         if extra_specs is None:
             extra_specs = {}
@@ -108,25 +138,70 @@ class TypesTest(utils.TestCase):
 
         expected_body["share_type"]["extra_specs"][
             "driver_handles_share_servers"] = dhss
+        expected_body["share_type"]["extra_specs"]['snapshot_support'] = (
+            True if snapshot is None else snapshot)
 
-        if spec_snapshot_support is None:
+        manager._create.assert_called_once_with(
-            if 'snapshot_support' not in extra_specs:
+            "/types", expected_body, "share_type")
-                expected_body["share_type"]["extra_specs"][
+        self.assertEqual("fake", result)
-                    'snapshot_support'] = True
+
+    def _add_standard_extra_specs_to_dict(self, extra_specs,
+                                          create_from_snapshot=None):
+
+        if all(spec is None for spec in [create_from_snapshot]):
+            return extra_specs
+
+        extra_specs = extra_specs or {}
+
+        if create_from_snapshot is not None:
+            extra_specs['create_share_from_snapshot_support'] = (
+                create_from_snapshot)
+
+        return extra_specs
+
+    @ddt.data(*get_valid_type_create_data_2_24())
+    @ddt.unpack
+    def test_create_2_24(self, is_public, dhss, snapshot, create_from_snapshot,
+                         extra_specs):
+
+        extra_specs = copy.copy(extra_specs)
+        extra_specs = self._add_standard_extra_specs_to_dict(
+            extra_specs, create_from_snapshot=create_from_snapshot)
+
+        manager = self._get_share_types_manager("2.24")
+        self.mock_object(manager, '_create', mock.Mock(return_value="fake"))
+
+        result = manager.create(
+            'test-type-3', spec_driver_handles_share_servers=dhss,
+            spec_snapshot_support=snapshot,
+            extra_specs=extra_specs, is_public=is_public)
+
+        expected_extra_specs = dict(extra_specs or {})
+        expected_extra_specs["driver_handles_share_servers"] = dhss
+
+        if snapshot is None:
+            expected_extra_specs.pop("snapshot_support", None)
         else:
-            expected_body["share_type"]["extra_specs"][
+            expected_extra_specs["snapshot_support"] = snapshot
-                'snapshot_support'] = spec_snapshot_support
 
-        with mock.patch.object(manager, '_create',
+        if create_from_snapshot is None:
-                               mock.Mock(return_value="fake")):
+            expected_extra_specs.pop("create_share_from_snapshot_support",
-            result = manager.create(
+                                     None)
-                'test-type-3', spec_driver_handles_share_servers=dhss,
+        else:
-                spec_snapshot_support=spec_snapshot_support,
+            expected_extra_specs["create_share_from_snapshot_support"] = (
-                extra_specs=extra_specs, is_public=is_public)
+                create_from_snapshot)
 
-            manager._create.assert_called_once_with(
+        expected_body = {
-                "/types", expected_body, "share_type")
+            "share_type": {
-            self.assertEqual("fake", result)
+                "name": 'test-type-3',
+                'share_type_access:is_public': is_public,
+                "extra_specs": expected_extra_specs,
+            }
+        }
+
+        manager._create.assert_called_once_with(
+            "/types", expected_body, "share_type")
+        self.assertEqual("fake", result)
 
     @ddt.data(
         (False, False, True, {'snapshot_support': True,
@@ -144,34 +219,67 @@ class TypesTest(utils.TestCase):
         (False, None, None, {'driver_handles_share_servers': None}),
     )
     @ddt.unpack
-    def test_create_command_error(self, is_public, dhss, spec_snapshot_support,
+    def test_create_error_2_7(self, is_public, dhss, snapshot,
-                                  extra_specs):
+                              extra_specs):
         manager = self._get_share_types_manager("2.7")
-        with mock.patch.object(manager, '_create',
+        self.mock_object(manager, '_create', mock.Mock(return_value="fake"))
-                               mock.Mock(return_value="fake")):
+
-            self.assertRaises(
+        self.assertRaises(
-                exceptions.CommandError,
+            exceptions.CommandError,
-                manager.create,
+            manager.create,
-                'test-type-3',
+            'test-type-3',
-                spec_driver_handles_share_servers=dhss,
+            spec_driver_handles_share_servers=dhss,
-                spec_snapshot_support=spec_snapshot_support,
+            spec_snapshot_support=snapshot,
-                extra_specs=extra_specs,
+            extra_specs=extra_specs,
-                is_public=is_public)
+            is_public=is_public)
+
+    @ddt.data(
+        (False, True, None, None, {'driver_handles_share_servers': True}),
+        (False, False, False, False, {'snapshot_support': True,
+                                      'replication_type': 'fake_repl_type'}),
+    )
+    @ddt.unpack
+    def test_create_error_2_24(self, is_public, dhss, snapshot,
+                               create_from_snapshot, extra_specs):
+
+        extra_specs = copy.copy(extra_specs)
+        extra_specs = self._add_standard_extra_specs_to_dict(
+            extra_specs, create_from_snapshot=create_from_snapshot)
+
+        manager = self._get_share_types_manager("2.24")
+        self.mock_object(manager, '_create', mock.Mock(return_value="fake"))
+
+        self.assertRaises(
+            exceptions.CommandError,
+            manager.create,
+            'test-type-3',
+            spec_driver_handles_share_servers=dhss,
+            spec_snapshot_support=snapshot,
+            extra_specs=extra_specs,
+            is_public=is_public)
 
     @ddt.data(
         ("2.6", True),
         ("2.7", True),
+        ("2.24", True),
         ("2.6", False),
         ("2.7", False),
+        ("2.24", False),
     )
     @ddt.unpack
     def test_create_with_default_values(self, microversion, dhss):
+
         manager = self._get_share_types_manager(microversion)
+        self.mock_object(manager, '_create', mock.Mock(return_value="fake"))
+
+        result = manager.create('test-type-3', dhss)
+
         if (api_versions.APIVersion(microversion) >
                 api_versions.APIVersion("2.6")):
             is_public_keyname = "share_type_access:is_public"
         else:
             is_public_keyname = "os-share-type-access:is_public"
+
         expected_body = {
             "share_type": {
                 "name": 'test-type-3',
@@ -183,13 +291,13 @@ class TypesTest(utils.TestCase):
             }
         }
 
-        with mock.patch.object(manager, '_create',
+        if (api_versions.APIVersion(microversion) >=
-                               mock.Mock(return_value="fake")):
+                api_versions.APIVersion("2.24")):
-            result = manager.create('test-type-3', dhss)
+            del expected_body['share_type']['extra_specs']['snapshot_support']
 
-            manager._create.assert_called_once_with(
+        manager._create.assert_called_once_with(
-                "/types", expected_body, "share_type")
+            "/types", expected_body, "share_type")
-            self.assertEqual("fake", result)
+        self.assertEqual("fake", result)
 
     def test_set_key(self):
         t = cs.share_types.get(1)

manilaclient/v2/share_types.py

@@ -133,73 +133,92 @@ class ShareTypeManager(base.ManagerWithFind):
         """
         self._delete("/types/%s" % common_base.getid(share_type))
 
-    def _do_create(self, name, spec_driver_handles_share_servers,
+    def _do_create(self, name, extra_specs, is_public,
-                   spec_snapshot_support, is_public=True,
+                   is_public_keyname="share_type_access:is_public"):
-                   is_public_keyname="os-share-type-access:is_public",
-                   optional_extra_specs=None):
         """Create a share type.
 
         :param name: Descriptive name of the share type
         :rtype: :class:`ShareType`
         """
-        if optional_extra_specs is None:
-            optional_extra_specs = {}
 
-        if spec_snapshot_support is not None:
+        body = {
-            if 'snapshot_support' in optional_extra_specs:
+            "share_type": {
-                msg = "'snapshot_support' extra spec is provided twice."
+                "name": name,
-                raise exceptions.CommandError(msg)
+                is_public_keyname: is_public,
-            else:
+                "extra_specs": extra_specs,
-                optional_extra_specs['snapshot_support'] = (
+            }
-                    spec_snapshot_support)
+        }
-        elif 'snapshot_support' not in optional_extra_specs:
+        return self._create("/types", body, "share_type")
-            optional_extra_specs['snapshot_support'] = True
+
+    @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):
+
+        if extra_specs is None:
+            extra_specs = {}
+
+        self._handle_spec_driver_handles_share_servers(
+            extra_specs, spec_driver_handles_share_servers)
+        self._handle_spec_snapshot_support(
+            extra_specs, spec_snapshot_support, set_default=True)
+
+        return self._do_create(
+            name, extra_specs, is_public,
+            is_public_keyname="os-share-type-access:is_public")
+
+    @api_versions.wraps("2.7", "2.23")  # noqa
+    def create(self, name, spec_driver_handles_share_servers,
+               spec_snapshot_support=None, is_public=True, extra_specs=None):
+
+        if extra_specs is None:
+            extra_specs = {}
+
+        self._handle_spec_driver_handles_share_servers(
+            extra_specs, spec_driver_handles_share_servers)
+        self._handle_spec_snapshot_support(
+            extra_specs, spec_snapshot_support, set_default=True)
+
+        return self._do_create(name, extra_specs, is_public)
+
+    @api_versions.wraps("2.24")  # noqa
+    def create(self, name, spec_driver_handles_share_servers,
+               spec_snapshot_support=None, is_public=True, extra_specs=None):
+
+        if extra_specs is None:
+            extra_specs = {}
+
+        self._handle_spec_driver_handles_share_servers(
+            extra_specs, spec_driver_handles_share_servers)
+        self._handle_spec_snapshot_support(extra_specs, spec_snapshot_support)
+
+        return self._do_create(name, extra_specs, is_public)
+
+    def _handle_spec_driver_handles_share_servers(
+            self, extra_specs, spec_driver_handles_share_servers):
+        """Validation and default for DHSS extra spec."""
 
         if spec_driver_handles_share_servers is not None:
-            if 'driver_handles_share_servers' in optional_extra_specs:
+            if 'driver_handles_share_servers' in extra_specs:
                 msg = ("'driver_handles_share_servers' is already set via "
                        "positional argument.")
                 raise exceptions.CommandError(msg)
             else:
-                optional_extra_specs['driver_handles_share_servers'] = (
+                extra_specs['driver_handles_share_servers'] = (
                     spec_driver_handles_share_servers)
         else:
             msg = ("'driver_handles_share_servers' is not set via "
                    "positional argument.")
             raise exceptions.CommandError(msg)
 
-        body = {
+    def _handle_spec_snapshot_support(self, extra_specs, spec_snapshot_support,
-            "share_type": {
+                                      set_default=False):
-                "name": name,
+        """Validation and default for snapshot extra spec."""
-                is_public_keyname: is_public,
-                "extra_specs": optional_extra_specs,
-            }
-        }
 
-        return self._create("/types", body, "share_type")
+        if spec_snapshot_support is not None:
-
+            if 'snapshot_support' in extra_specs:
-    @api_versions.wraps("1.0", "2.6")
+                msg = "'snapshot_support' extra spec is provided twice."
-    def create(self, name, spec_driver_handles_share_servers,
+                raise exceptions.CommandError(msg)
-               spec_snapshot_support=None, is_public=True,
+            else:
-               extra_specs=None):
+                extra_specs['snapshot_support'] = spec_snapshot_support
-
+        elif 'snapshot_support' not in extra_specs and set_default:
-        return self._do_create(
+            extra_specs['snapshot_support'] = True
-            name,
-            spec_driver_handles_share_servers,
-            spec_snapshot_support,
-            is_public,
-            "os-share-type-access:is_public",
-            optional_extra_specs=extra_specs)
-
-    @api_versions.wraps("2.7")  # noqa
-    def create(self, name, spec_driver_handles_share_servers,
-               spec_snapshot_support=None, is_public=True,
-               extra_specs=None):
-
-        return self._do_create(
-            name,
-            spec_driver_handles_share_servers,
-            spec_snapshot_support,
-            is_public,
-            "share_type_access:is_public",
-            optional_extra_specs=extra_specs)

manilaclient/v2/shell.py

@@ -2876,8 +2876,15 @@ def do_extra_specs_list(cs, args):
     '--snapshot-support',
     metavar='<snapshot_support>',
     action='single_alias',
-    help="Boolean extra spec that used for filtering of back ends by their "
+    help="Boolean extra spec used for filtering of back ends by their "
-         "capability to create share snapshots. (Default is True).")
+         "capability to create share snapshots.")
+@cliutils.arg(
+    '--create_share_from_snapshot_support',
+    '--create-share-from-snapshot-support',
+    metavar='<create_share_from_snapshot_support>',
+    action='single_alias',
+    help="Boolean extra spec used for filtering of back ends by their "
+         "capability to create shares from snapshots.")
 @cliutils.arg(
     '--extra-specs',
     '--extra_specs',  # alias
@@ -2918,22 +2925,26 @@ def do_type_create(cs, args):
                "set via positional argument.")
         raise exceptions.CommandError(msg)
 
-    if args.snapshot_support and 'snapshot_support' in kwargs['extra_specs']:
+    boolean_keys = ('snapshot_support', 'create_share_from_snapshot_support')
-        msg = ("Argument 'snapshot_support' value specified twice.")
+    for key in boolean_keys:
-        raise exceptions.CommandError(msg)
+        value = getattr(args, key)
 
-    try:
+        if value is not None and key in kwargs['extra_specs']:
-        if args.snapshot_support:
+            msg = ("Argument '%s' value specified twice." % key)
-            kwargs['spec_snapshot_support'] = strutils.bool_from_string(
+            raise exceptions.CommandError(msg)
-                args.snapshot_support, strict=True)
+
-        elif 'snapshot_support' in kwargs['extra_specs']:
+        try:
-            kwargs['extra_specs']['snapshot_support'] = (
+            if value:
-                strutils.bool_from_string(
+                kwargs['extra_specs'][key] = (
-                    kwargs['extra_specs']['snapshot_support'], strict=True))
+                    strutils.bool_from_string(value, strict=True))
-    except ValueError as e:
+            elif key in kwargs['extra_specs']:
-        msg = ("Argument 'snapshot_support' is of boolean type and has "
+                kwargs['extra_specs'][key] = (
-               "invalid value: %s" % six.text_type(e))
+                    strutils.bool_from_string(
-        raise exceptions.CommandError(msg)
+                        kwargs['extra_specs'][key], strict=True))
+        except ValueError as e:
+            msg = ("Argument '%s' is of boolean "
+                   "type and has invalid value: %s" % (key, six.text_type(e)))
+            raise exceptions.CommandError(msg)
 
     stype = cs.share_types.create(**kwargs)
     _print_share_type(stype)

releasenotes/notes/support-create-share-from-snapshot-extra-spec-cdba92f179c1c5c6.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - Support the new optional create_share_from_snapshot_support extra spec,
+    and handle the newly-optional snapshot_support extra spec.