openstack
/
python-manilaclient
Code Issues Proposed changes

Merge "type-create should support specifying extra-specs"

This commit is contained in:
Jenkins 2016-09-01 18:06:08 +00:00 committed by Gerrit Code Review
parent b5b1ebed62 efeb9dba99
commit ca9bd141d8
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

@ -163,8 +163,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(
@ -173,6 +174,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

@ -144,8 +144,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
@ -156,22 +156,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

@ -685,11 +685,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):
@ -728,12 +728,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

@ -254,6 +254,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:
@ -265,7 +267,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
@ -2769,6 +2779,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',
@ -2789,10 +2811,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.