openstack/python-manilaclient
Code Issues Proposed changes

type-create should support specifying extra-specs

Browse Source 
The share-types POST API in Manila accepts a dictionary of extra-specs.
However, the CLI only accepts values for DHSS and snapshot_support.
Fix is to make provision of specifying more extra-specs in type_create.

Change-Id: I4a0fbf572653d331f6351bc07bfdd58bbc5a74b5
Closes-Bug: #1561171
This commit is contained in:
nidhimittalhada 2016-05-09 16:22:44 +05:30
parent 22172b65f8
commit efeb9dba99
10 changed files with 318 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
manilaclient/tests/functional/base.py
View File

@ -158,8 +158,9 @@ 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, microversion=None):
snapshot_support=None, is_public=True, 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(
@ -168,6 +169,7 @@ class BaseTestCase(base.ClientTestBase):
snapshot_support=snapshot_support,
is_public=is_public,
microversion=microversion,
extra_specs=extra_specs,
)
resource = {
"type": "share_type",

30
manilaclient/tests/functional/client.py
View File

@ -142,8 +142,8 @@ class ManilaCLIClient(base.CLIClient):
# Share types
def create_share_type(self, name=None, driver_handles_share_servers=True,
snapshot_support=True, is_public=True,
microversion=None):
snapshot_support=None, is_public=True,
microversion=None, extra_specs=None):
"""Creates share type.
:param name: text -- name of share type to use, if not set then
@ -154,22 +154,34 @@ class ManilaCLIClient(base.CLIClient):
string alias. Default is True.
:param is_public: bool/str -- boolean or its string alias. Default is
True.
:param extra_specs: -- dictionary of extra specs Default is None.
"""
if name is None:
name = data_utils.rand_name('manilaclient_functional_test')
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 '
'--snapshot-support %(snapshot_support)s') % {
'name': name, 'dhss': dhss, 'is_public': is_public,
'snapshot_support': snapshot_support}
cmd = ('type-create %(name)s %(dhss)s --is-public %(is_public)s ') % {
'name': name, 'dhss': dhss, 'is_public': is_public}
if snapshot_support is not None:
if not isinstance(snapshot_support, six.string_types):
snapshot_support = six.text_type(snapshot_support)
cmd += " --snapshot-support " + snapshot_support
if extra_specs is not None:
extra_spec_str = ''
for k, v in extra_specs.items():
if not isinstance(v, six.string_types):
extra_specs[k] = six.text_type(v)
extra_spec_str += "{}='{}' ".format(k, v)
cmd += " --extra_specs " + extra_spec_str
share_type_raw = self.manila(cmd, microversion=microversion)
share_type = output_parser.details(share_type_raw)
share_type = utils.details(share_type_raw)
return share_type
@not_found_wrapper

62
manilaclient/tests/functional/test_share_types.py
View File

@ -81,42 +81,76 @@ class ShareTypesReadWriteTest(base.BaseTestCase):
microversion=microversion))
@ddt.data(
(True, False),
(True, True),
(False, True),
(False, False),
(False, True, False, "2.6"),
(False, False, True, "2.7"),
(True, False, None),
(True, True, None),
(False, True, None),
(False, False, None),
(False, True, None, {'snapshot_support': False,
'replication_type': 'fake_repl_type1'}, "2.6"),
(False, False, None, {'snapshot_support': True,
'replication_type': 'fake_repl_type2'}, "2.7"),
(False, False, None, {'snapshot_support': False,
'replication_type': 'fake_repl_type3',
'foo': 'bar'}),
(False, False, None, {'replication_type': 'fake_repl_type4',
'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,
snapshot_support=True,
microversion=None):
spec_snapshot_support=None,
extra_specs=None, microversion=None):
if microversion:
self.skip_if_microversion_not_supported(microversion)
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
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=snapshot_support,
snapshot_support=spec_snapshot_support,
is_public=is_public,
microversion=microversion,
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)
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'])
if not isinstance(optional_extra_specs, list):
optional_extra_specs = [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)
self.assertEqual('public' if is_public else 'private',
share_type['Visibility'].lower())
self.assertEqual('-', share_type['is_default'])

9
manilaclient/tests/functional/utils.py
View File

@ -93,6 +93,15 @@ def listing(output_lines):
return items
def details(output_lines):
"""Returns dict parsed from CLI output."""
result = listing(output_lines)
d = {}
for item in result:
d.update({item['Property']: item['Value']})
return d
def is_microversion_supported(microversion):
return (
api_versions.APIVersion(CONF.min_api_microversion) <=

10
manilaclient/tests/unit/v2/fakes.py
View File

@ -672,11 +672,11 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
return (200, {}, {
'share_types': [{'id': 1,
'name': 'test-type-1',
'extra_specs': {'test': 'test'},
'extra_specs': {'test1': 'test1'},
'required_extra_specs': {'test': 'test'}},
{'id': 2,
'name': 'test-type-2',
'extra_specs': {'test': 'test'},
'extra_specs': {'test1': 'test1'},
'required_extra_specs': {'test': 'test'}}]})
def get_types_1(self, **kw):
@ -715,12 +715,16 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
def post_types(self, body, **kw):
share_type = body['share_type']
required_extra_specs = {
"driver_handles_share_servers": share_type[
'extra_specs']['driver_handles_share_servers'],
}
return (202, {}, {
'share_type': {
'id': 3,
'name': 'test-type-3',
'extra_specs': share_type['extra_specs'],
'required_extra_specs': share_type['extra_specs'],
'required_extra_specs': required_extra_specs,
}
})

62
manilaclient/tests/unit/v2/test_shell.py
View File

@ -756,6 +756,32 @@ class ShellTest(test_utils.TestCase):
'type-create test ' + value,
)
@ddt.data('True', 'False')
def test_type_create_duplicate_dhss(self, value):
self.assertRaises(
exceptions.CommandError,
self.run_command,
'type-create test ' + value +
' --extra-specs driver_handles_share_servers=' + value,
)
@ddt.data('True', 'False')
def test_type_create_duplicate_snapshot_support(self, value):
self.assertRaises(
exceptions.CommandError,
self.run_command,
'type-create test True --snapshot-support ' + value +
' --extra-specs snapshot_support=' + value,
)
def test_type_create_duplicate_extra_spec_key(self):
self.assertRaises(
exceptions.CommandError,
self.run_command,
'type-create test True --extra-specs'
' a=foo1 a=foo2',
)
@ddt.unpack
@ddt.data({'expected_bool': True, 'text': 'true'},
{'expected_bool': True, 'text': '1'},
@ -785,6 +811,29 @@ class ShellTest(test_utils.TestCase):
for v in ('false', 'False', '0', 'FALSE', 'fAlSe')])
)
def test_create_with_snapshot_support(self, expected_bool, text):
expected = {
"share_type": {
"name": "test",
"share_type_access:is_public": True,
"extra_specs": {
"snapshot_support": expected_bool,
"driver_handles_share_servers": False,
}
}
}
self.run_command('type-create test false --snapshot-support ' + text)
self.assert_called('POST', '/types', body=expected)
@ddt.unpack
@ddt.data({'expected_bool': True,
'snapshot_text': 'true',
'replication_type': 'readable'},
{'expected_bool': False,
'snapshot_text': 'false',
'replication_type': 'writable'})
def test_create_with_extra_specs(self, expected_bool, snapshot_text,
replication_type):
expected = {
"share_type": {
"name": "test",
@ -792,11 +841,14 @@ class ShellTest(test_utils.TestCase):
"extra_specs": {
"driver_handles_share_servers": False,
"snapshot_support": expected_bool,
"replication_type": replication_type,
}
}
}
self.run_command('type-create test false --snapshot-support ' + text)
self.run_command('type-create test false --extra-specs'
' snapshot_support=' + snapshot_text +
' replication_type=' + replication_type)
self.assert_called('POST', '/types', body=expected)
@ -808,6 +860,14 @@ class ShellTest(test_utils.TestCase):
'type-create test false --snapshot-support ' + value,
)
@ddt.data('fake', 'FFFalse', 'trueee')
def test_type_create_invalid_snapshot_support_value2(self, value):
self.assertRaises(
exceptions.CommandError,
self.run_command,
'type-create test false --extra-specs snapshot_support=' + value,
)
@ddt.data('--is-public', '--is_public')
def test_update(self, alias):
# basic rename with positional arguments

102
manilaclient/tests/unit/v2/test_types.py
View File

@ -15,6 +15,7 @@ import ddt
import mock
from manilaclient import api_versions
from manilaclient import exceptions
from manilaclient.tests.unit import utils
from manilaclient.tests.unit.v2 import fakes
from manilaclient.v2 import share_types
@ -35,7 +36,7 @@ class TypesTest(utils.TestCase):
{'snapshot_support': 'False', 'foo': 'bar'},
)
def test_init(self, extra_specs):
info = {'extra_specs': {'snapshot_support': 'False'}}
info = {'extra_specs': extra_specs}
share_type = share_types.ShareType(share_types.ShareTypeManager, info)
@ -43,8 +44,7 @@ class TypesTest(utils.TestCase):
self.assertTrue(hasattr(share_type, '_optional_extra_specs'))
self.assertIsInstance(share_type._required_extra_specs, dict)
self.assertIsInstance(share_type._optional_extra_specs, dict)
self.assertEqual(
{'snapshot_support': 'False'}, share_type.get_optional_keys())
self.assertEqual(extra_specs, share_type.get_optional_keys())
def test_list_types(self):
tl = cs.share_types.list()
@ -54,46 +54,110 @@ class TypesTest(utils.TestCase):
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())
self.assertEqual({'test1': 'test1'}, 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, True, True),
(True, True, False),
(True, False, True),
(False, True, True),
(True, False, False),
(False, False, True),
(False, True, False),
(False, False, False),
(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, snapshot_support):
def test_create(self, is_public, dhss, spec_snapshot_support, extra_specs):
manager = self._get_share_types_manager("2.7")
if extra_specs is None:
extra_specs = {}
expected_extra_specs = dict(extra_specs)
expected_body = {
"share_type": {
"name": 'test-type-3',
'share_type_access:is_public': is_public,
"extra_specs": {
"driver_handles_share_servers": dhss,
"snapshot_support": snapshot_support,
}
"extra_specs": expected_extra_specs,
}
}
expected_body["share_type"]["extra_specs"][
"driver_handles_share_servers"] = dhss
if spec_snapshot_support is None:
if 'snapshot_support' not in extra_specs:
expected_body["share_type"]["extra_specs"][
'snapshot_support'] = True
else:
expected_body["share_type"]["extra_specs"][
'snapshot_support'] = spec_snapshot_support
with mock.patch.object(manager, '_create',
mock.Mock(return_value="fake")):
result = manager.create(
'test-type-3', dhss, snapshot_support, is_public=is_public)
'test-type-3', spec_driver_handles_share_servers=dhss,
spec_snapshot_support=spec_snapshot_support,
extra_specs=extra_specs, is_public=is_public)
manager._create.assert_called_once_with(
"/types", expected_body, "share_type")
self.assertEqual("fake", result)
@ddt.data(
(False, False, True, {'snapshot_support': True,
'replication_type': 'fake_repl_type'}),
(False, False, False, {'snapshot_support': False,
'replication_type': 'fake_repl_type'}),
(False, False, True, {'snapshot_support': False,
'replication_type': 'fake_repl_type'}),
(False, False, False, {'snapshot_support': True,
'replication_type': 'fake_repl_type'}),
(False, True, None, {'driver_handles_share_servers': True}),
(False, False, None, {'driver_handles_share_servers': True}),
(False, None, None, {'driver_handles_share_servers': True}),
(False, None, None, {'driver_handles_share_servers': None}),
)
@ddt.unpack
def test_create_command_error(self, is_public, dhss, spec_snapshot_support,
extra_specs):
manager = self._get_share_types_manager("2.7")
with mock.patch.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=spec_snapshot_support,
extra_specs=extra_specs,
is_public=is_public)
@ddt.data(
("2.6", True),
("2.7", True),

58
manilaclient/v2/share_types.py
View File

@ -20,6 +20,7 @@ Share Type interface.
from manilaclient import api_versions
from manilaclient import base
from manilaclient import exceptions
from manilaclient.openstack.common.apiclient import base as common_base
@ -29,10 +30,9 @@ 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'),
}
self._optional_extra_specs = info.get('extra_specs', {}).copy()
for key in self._required_extra_specs.keys():
self._optional_extra_specs.pop(key, None)
def __repr__(self):
return "<ShareType: %s>" % self.name
@ -134,23 +134,45 @@ class ShareTypeManager(base.ManagerWithFind):
self._delete("/types/%s" % common_base.getid(share_type))
def _do_create(self, name, spec_driver_handles_share_servers,
spec_snapshot_support=True, is_public=True,
is_public_keyname="os-share-type-access:is_public"):
spec_snapshot_support, is_public=True,
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:
if 'snapshot_support' in optional_extra_specs:
msg = "'snapshot_support' extra spec is provided twice."
raise exceptions.CommandError(msg)
else:
optional_extra_specs['snapshot_support'] = (
spec_snapshot_support)
elif 'snapshot_support' not in optional_extra_specs:
optional_extra_specs['snapshot_support'] = True
if spec_driver_handles_share_servers is not None:
if 'driver_handles_share_servers' in optional_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'] = (
spec_driver_handles_share_servers)
else:
msg = ("'driver_handles_share_servers' is not set via "
"positional argument.")
raise exceptions.CommandError(msg)
body = {
"share_type": {
"name": name,
is_public_keyname: is_public,
"extra_specs": {
"driver_handles_share_servers":
spec_driver_handles_share_servers,
"snapshot_support": spec_snapshot_support,
},
"extra_specs": optional_extra_specs,
}
}
@ -158,20 +180,26 @@ class ShareTypeManager(base.ManagerWithFind):
@api_versions.wraps("1.0", "2.6")
def create(self, name, spec_driver_handles_share_servers,
spec_snapshot_support=True, is_public=True):
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,
"os-share-type-access: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=True, is_public=True):
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")
"share_type_access:is_public",
optional_extra_specs=extra_specs)

40
manilaclient/v2/shell.py
View File

@ -248,6 +248,8 @@ def _extract_extra_specs(args):
def _extract_key_value_options(args, option_name):
result_dict = {}
duplicate_options = []
options = getattr(args, option_name, None)
if options:
@ -259,7 +261,15 @@ def _extract_key_value_options(args, option_name):
key = option
value = None
result_dict[key] = value
if key not in result_dict:
result_dict[key] = value
else:
duplicate_options.append(key)
if len(duplicate_options) > 0:
duplicate_str = ', '.join(duplicate_options)
msg = "Following options were duplicated: %s" % duplicate_str
raise exceptions.CommandError(msg)
return result_dict
@ -2633,6 +2643,18 @@ def do_extra_specs_list(cs, args):
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(
'--extra-specs',
'--extra_specs', # alias
type=str,
nargs='*',
metavar='<key=value>',
action='single_alias',
help="Extra specs key and value of share type that will be"
" used for share type creation. OPTIONAL: Default=None."
" e.g --extra-specs thin_provisioning='<is> True', "
"replication_type=readable.",
default=None)
@cliutils.arg(
'--is_public',
'--is-public',
@ -2653,10 +2675,26 @@ def do_type_create(cs, args):
msg = ("Argument spec_driver_handles_share_servers "
"argument is not valid: %s" % six.text_type(e))
raise exceptions.CommandError(msg)
kwargs['extra_specs'] = _extract_extra_specs(args)
if 'driver_handles_share_servers' in kwargs['extra_specs']:
msg = ("Argument 'driver_handles_share_servers' is already "
"set via positional argument.")
raise exceptions.CommandError(msg)
if args.snapshot_support and 'snapshot_support' in kwargs['extra_specs']:
msg = ("Argument 'snapshot_support' value specified twice.")
raise exceptions.CommandError(msg)
try:
if args.snapshot_support:
kwargs['spec_snapshot_support'] = strutils.bool_from_string(
args.snapshot_support, strict=True)
elif 'snapshot_support' in kwargs['extra_specs']:
kwargs['extra_specs']['snapshot_support'] = (
strutils.bool_from_string(
kwargs['extra_specs']['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))

3
releasenotes/notes/type-create-extra-specs-691572762357ef3b.yaml Normal file
View File

@ -0,0 +1,3 @@
---
fixes:
- User can specify any number of extra-specs in type_create.