openstack/python-manilaclient
Code Issues Proposed changes

Rename volume_type to share_type

Browse Source 
This change renames support for volume_type to share_type.
The manila client will send data to Manila API using share_type, but
passes both share_type and volume_type values returned from API call to users
of the python binding; however, CLI output will only print the value of
share_type. This change ensures that --volume-type (and its other
permutations) are aliases of --share-type, thus providing backwards
compatibility for command line arguments of the shell.

Change-Id: Id40eea27c2c5b44e1d01c061a48e1658ba800f43
This commit is contained in:
Bob Callaway 2015-02-15 14:59:07 -08:00
parent f24b1e4037
commit f4ddf3626c
10 changed files with 130 additions and 124 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
manilaclient/common/constants.py
View File

@ -23,7 +23,7 @@ SHARE_SORT_KEY_VALUES = (
'user_id', 'project_id',
'created_at', 'updated_at',
'display_name', 'name',
'volume_type_id', 'volume_type',
'share_type_id', 'share_type',
'share_network_id', 'share_network',
'snapshot_id', 'snapshot',
)

6
manilaclient/tests/functional/test_volume_types.py → manilaclient/tests/functional/test_share_types.py
View File

@ -19,12 +19,12 @@ from manilaclient.tests.functional import base
@ddt.ddt
class ManilaClientTestVolumeTypesReadOnly(base.BaseTestCase):
class ManilaClientTestShareTypesReadOnly(base.BaseTestCase):
@ddt.data('admin', 'user')
def test_volume_type_list(self, role):
def test_share_type_list(self, role):
self.clients[role].manila('type-list')
@ddt.data('admin', 'user')
@ddt.data('admin')
def test_extra_specs_list(self, role):
self.clients[role].manila('extra-specs-list')

18
manilaclient/tests/unit/v1/fakes.py
View File

@ -268,23 +268,23 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
def get_types(self, **kw):
return (200, {}, {
'volume_types': [{'id': 1,
'name': 'test-type-1',
'extra_specs': {}},
{'id': 2,
'name': 'test-type-2',
'extra_specs': {}}]})
'share_types': [{'id': 1,
'name': 'test-type-1',
'extra_specs': {}},
{'id': 2,
'name': 'test-type-2',
'extra_specs': {}}]})
def get_types_1(self, **kw):
return (200, {}, {'volume_type': {
return (200, {}, {'share_type': {
'id': 1, 'name': 'test-type-1', 'extra_specs': {}}})
def get_types_2(self, **kw):
return (200, {}, {'volume_type': {
return (200, {}, {'share_type': {
'id': 2, 'name': 'test-type-2', 'extra_specs': {}}})
def post_types(self, body, **kw):
return (202, {}, {'volume_type': {
return (202, {}, {'share_type': {
'id': 3, 'name': 'test-type-3', 'extra_specs': {}}})
def post_types_1_extra_specs(self, body, **kw):

6
manilaclient/tests/unit/v1/test_shares.py
View File

@ -135,9 +135,9 @@ class SharesTest(utils.TestCase):
cs.shares.list(detailed=False, sort_key='snapshot')
cs.assert_called('GET', '/shares?sort_key=snapshot_id')
def test_list_shares_filter_by_volume_type_alias(self):
cs.shares.list(detailed=False, sort_key='volume_type')
cs.assert_called('GET', '/shares?sort_key=volume_type_id')
def test_list_shares_filter_by_share_type_alias(self):
cs.shares.list(detailed=False, sort_key='share_type')
cs.assert_called('GET', '/shares?sort_key=share_type_id')
def test_list_shares_by_improper_direction(self):
self.assertRaises(ValueError, cs.shares.list, sort_dir='fake')

26
manilaclient/tests/unit/v1/test_shell.py
View File

@ -137,28 +137,28 @@ class ShellTest(test_utils.TestCase):
'/shares/detail?extra_specs=%7B%27key%27%3A+%27value%27%7D',
)
def test_list_filter_by_volume_type_and_its_aliases(self):
fake_vt = type('Empty', (object,), {'id': 'fake_vt'})
def test_list_filter_by_share_type_and_its_aliases(self):
fake_st = type('Empty', (object,), {'id': 'fake_st'})
aliases = [
'--volume-type', '--volume_type', '--volume-type-id',
'--volume-type_id', '--volume_type-id', '--volume_type_id',
'--share-type', '--share_type', '--share-type-id',
'--share-type_id', '--share_type-id', '--share_type_id',
]
for alias in aliases:
for separator in self.separators:
with mock.patch.object(
apiclient_utils,
'find_resource',
mock.Mock(return_value=fake_vt)):
self.run_command('list ' + alias + separator + fake_vt.id)
mock.Mock(return_value=fake_st)):
self.run_command('list ' + alias + separator + fake_st.id)
self.assert_called(
'GET', '/shares/detail?volume_type_id=' + fake_vt.id)
'GET', '/shares/detail?share_type_id=' + fake_st.id)
def test_list_filter_by_volume_type_not_found(self):
def test_list_filter_by_share_type_not_found(self):
for separator in self.separators:
self.assertRaises(
exceptions.CommandError,
self.run_command,
'list --volume-type' + separator + 'not_found_expected',
'list --share-type' + separator + 'not_found_expected',
)
self.assert_called('GET', '/types')
@ -197,7 +197,7 @@ class ShellTest(test_utils.TestCase):
self.run_command('list ' + alias + separator + key)
key = 'share_network_id' if key == 'share_network' else key
key = 'snapshot_id' if key == 'snapshot' else key
key = 'volume_type_id' if key == 'volume_type' else key
key = 'share_type_id' if key == 'share_type' else key
self.assert_called('GET', '/shares/detail?sort_key=' + key)
def test_list_with_fake_sort_key(self):
@ -700,7 +700,7 @@ class ShellTest(test_utils.TestCase):
self.run_command("create nfs 1")
expected = {
"share": {
"volume_type": None,
"share_type": None,
"name": None,
"snapshot_id": None,
"description": None,
@ -720,7 +720,7 @@ class ShellTest(test_utils.TestCase):
self.run_command("create nfs 1 --share-network %s" % sn)
expected = {
"share": {
"volume_type": None,
"share_type": None,
"name": None,
"snapshot_id": None,
"description": None,
@ -738,7 +738,7 @@ class ShellTest(test_utils.TestCase):
self.run_command("create nfs 1 --metadata key1=value1 key2=value2")
expected = {
"share": {
"volume_type": None,
"share_type": None,
"name": None,
"snapshot_id": None,
"description": None,

16
manilaclient/tests/unit/v1/test_types.py
View File

@ -13,35 +13,35 @@
from manilaclient.tests.unit import utils
from manilaclient.tests.unit.v1 import fakes
from manilaclient.v1 import volume_types
from manilaclient.v1 import share_types
cs = fakes.FakeClient()
class TypesTest(utils.TestCase):
def test_list_types(self):
tl = cs.volume_types.list()
tl = cs.share_types.list()
cs.assert_called('GET', '/types')
for t in tl:
self.assertIsInstance(t, volume_types.VolumeType)
self.assertIsInstance(t, share_types.ShareType)
def test_create(self):
t = cs.volume_types.create('test-type-3')
t = cs.share_types.create('test-type-3')
cs.assert_called('POST', '/types')
self.assertIsInstance(t, volume_types.VolumeType)
self.assertIsInstance(t, share_types.ShareType)
def test_set_key(self):
t = cs.volume_types.get(1)
t = cs.share_types.get(1)
t.set_keys({'k': 'v'})
cs.assert_called('POST',
'/types/1/extra_specs',
{'extra_specs': {'k': 'v'}})
def test_unsset_keys(self):
t = cs.volume_types.get(1)
t = cs.share_types.get(1)
t.unset_keys(['k'])
cs.assert_called('DELETE', '/types/1/extra_specs/k')
def test_delete(self):
cs.volume_types.delete(1)
cs.share_types.delete(1)
cs.assert_called('DELETE', '/types/1')

4
manilaclient/v1/client.py
View File

@ -19,8 +19,8 @@ from manilaclient.v1 import services
from manilaclient.v1 import share_networks
from manilaclient.v1 import share_servers
from manilaclient.v1 import share_snapshots
from manilaclient.v1 import share_types
from manilaclient.v1 import shares
from manilaclient.v1 import volume_types
class Client(object):
@ -58,7 +58,7 @@ class Client(object):
self.shares = shares.ShareManager(self)
self.share_snapshots = share_snapshots.ShareSnapshotManager(self)
self.volume_types = volume_types.VolumeTypeManager(self)
self.share_types = share_types.ShareTypeManager(self)
self.share_servers = share_servers.ShareServerManager(self)
# Add in any extensions...

62
manilaclient/v1/volume_types.py → manilaclient/v1/share_types.py
View File

@ -15,32 +15,32 @@
"""
Volume Type interface.
Share Type interface.
"""
from manilaclient import base
from manilaclient.openstack.common.apiclient import base as common_base
class VolumeType(common_base.Resource):
"""A Volume Type is the type of volume to be created."""
class ShareType(common_base.Resource):
"""A Share Type is the type of share to be created."""
def __repr__(self):
return "<VolumeType: %s>" % self.name
return "<ShareType: %s>" % self.name
def get_keys(self):
"""Get extra specs from a volume type.
"""Get extra specs from a share type.
:param vol_type: The :class:`VolumeType` to get extra specs from
:param share_type: The :class:`ShareType` to get extra specs from
"""
_resp, body = self.manager.api.client.get(
"/types/%s/extra_specs" % common_base.getid(self))
return body["extra_specs"]
def set_keys(self, metadata):
"""Set extra specs on a volume type.
"""Set extra specs on a share type.
:param type : The :class:`VolumeType` to set extra spec on
:param type : The :class:`ShareType` to set extra spec on
:param metadata: A dict of key/value pairs to be set
"""
body = {'extra_specs': metadata}
@ -52,9 +52,9 @@ class VolumeType(common_base.Resource):
)
def unset_keys(self, keys):
"""Unset extra specs on a volume type.
"""Unset extra specs on a share type.
:param type_id: The :class:`VolumeType` to unset extra spec on
:param type_id: The :class:`ShareType` to unset extra spec on
:param keys: A list of keys to be unset
"""
@ -70,45 +70,45 @@ class VolumeType(common_base.Resource):
return resp
class VolumeTypeManager(base.ManagerWithFind):
"""Manage :class:`VolumeType` resources."""
class ShareTypeManager(base.ManagerWithFind):
"""Manage :class:`ShareType` resources."""
resource_class = VolumeType
resource_class = ShareType
def list(self, search_opts=None):
"""Get a list of all volume types.
"""Get a list of all share types.
:rtype: list of :class:`VolumeType`.
:rtype: list of :class:`ShareType`.
"""
return self._list("/types", "volume_types")
return self._list("/types", "share_types")
def get(self, volume_type):
"""Get a specific volume type.
def get(self, share_type):
"""Get a specific share type.
:param volume_type: The ID of the :class:`VolumeType` to get.
:rtype: :class:`VolumeType`
:param share_type: The ID of the :class:`ShareType` to get.
:rtype: :class:`ShareType`
"""
return self._get("/types/%s" % common_base.getid(volume_type),
"volume_type")
return self._get("/types/%s" % common_base.getid(share_type),
"share_type")
def delete(self, volume_type):
"""Delete a specific volume_type.
def delete(self, share_type):
"""Delete a specific share_type.
:param volume_type: The name or ID of the :class:`VolumeType` to get.
:param share_type: The name or ID of the :class:`ShareType` to get.
"""
self._delete("/types/%s" % common_base.getid(volume_type))
self._delete("/types/%s" % common_base.getid(share_type))
def create(self, name):
"""Create a volume type.
"""Create a share type.
:param name: Descriptive name of the volume type
:rtype: :class:`VolumeType`
:param name: Descriptive name of the share type
:rtype: :class:`ShareType`
"""
body = {
"volume_type": {
"share_type": {
"name": name,
}
}
return self._create("/types", body, "volume_type")
return self._create("/types", body, "share_type")

16
manilaclient/v1/shares.py
View File

@ -129,7 +129,7 @@ class ShareManager(base.ManagerWithFind):
def create(self, share_proto, size, snapshot_id=None, name=None,
description=None, metadata=None, share_network=None,
volume_type=None):
share_type=None):
"""Create NAS.
:param size: Size of NAS in GB
@ -137,7 +137,7 @@ class ShareManager(base.ManagerWithFind):
:param name: Name of the NAS
:param description: Short description of a share
:param share_proto: Type of NAS (NFS, CIFS, GlusterFS or HDFS)
:param metadata: Optional metadata to set on volume creation
:param metadata: Optional metadata to set on share creation
:rtype: :class:`Share`
"""
@ -153,13 +153,13 @@ class ShareManager(base.ManagerWithFind):
'metadata': share_metadata,
'share_proto': share_proto,
'share_network_id': common_base.getid(share_network),
'volume_type': volume_type}}
'share_type': share_type}}
return self._create('/shares', body, 'share')
def get(self, share_id):
"""Get a share.
:param share_id: The ID of the share to delete.
:param share_id: The ID of the share to get.
:rtype: :class:`Share`
"""
return self._get("/shares/%s" % share_id, "share")
@ -193,8 +193,8 @@ class ShareManager(base.ManagerWithFind):
if sort_key in constants.SHARE_SORT_KEY_VALUES:
search_opts['sort_key'] = sort_key
# NOTE(vponomaryov): Replace aliases with appropriate keys
if sort_key == 'volume_type':
search_opts['sort_key'] = 'volume_type_id'
if sort_key == 'share_type':
search_opts['sort_key'] = 'share_type_id'
elif sort_key == 'snapshot':
search_opts['sort_key'] = 'snapshot_id'
elif sort_key == 'share_network':
@ -290,7 +290,7 @@ class ShareManager(base.ManagerWithFind):
body, "metadata")
def delete_metadata(self, share, keys):
"""Delete specified keys from volumes metadata.
"""Delete specified keys from shares metadata.
:param share: The :class:`Share`.
:param keys: A list of keys to be removed.
@ -302,7 +302,7 @@ class ShareManager(base.ManagerWithFind):
def update_all_metadata(self, share, metadata):
"""Update all metadata of a share.
:param share: The :class:`Volume`.
:param share: The :class:`Share`.
:param metadata: A list of keys to be updated.
"""
body = {'metadata': metadata}

98
manilaclient/v1/shell.py
View File

@ -64,6 +64,9 @@ def _find_share(cs, share):
def _print_share(cs, share):
info = share._info.copy()
info.pop('links', None)
# No need to print both volume_type and share_type to CLI
if 'volume_type' in info and 'share_type' in info:
info.pop('volume_type', None)
cliutils.print_dict(info)
@ -369,10 +372,12 @@ def do_rate_limits(cs, args):
help='Optional share description. (Default=None)',
default=None)
@cliutils.arg(
'--volume-type',
metavar='<volume-type>',
help='Optional volume type. (Default=None)',
default=None)
'--share-type', '--share_type', '--volume-type', '--volume_type',
metavar='<share-type>',
default=None,
action='single_alias',
help='Optional share type. Use of optional volume type is deprecated'
'(Default=None)')
@cliutils.service_type('share')
def do_create(cs, args):
"""Creates a new share (NFS, CIFS, GlusterFS or HDFS)."""
@ -388,7 +393,7 @@ def do_create(cs, args):
args.name, args.description,
metadata=share_metadata,
share_network=share_network,
volume_type=args.volume_type)
share_type=args.share_type)
_print_share(cs, share)
@ -606,18 +611,19 @@ def do_access_list(cs, args):
nargs='*',
metavar='<key=value>',
action='single_alias',
help='Filters results by a extra specs key and value of volume type that '
help='Filters results by a extra specs key and value of share type that '
'was used for share creation. OPTIONAL: Default=None',
default=None)
@cliutils.arg(
'--volume-type',
'--volume_type', '--volume-type-id', # aliases
'--volume-type_id', '--volume_type-id', '--volume_type_id', # aliases
metavar='<volume_type>',
'--share-type', '--volume-type'
'--share_type', '--share-type-id', '--volume-type-id', # aliases
'--share-type_id', '--share_type-id', '--share_type_id', # aliases
'--volume_type', '--volume_type_id',
metavar='<share_type>',
type=str,
default=None,
action='single_alias',
help='Filter results by a volume type id or name that was used for share '
help='Filter results by a share type id or name that was used for share '
'creation.')
@cliutils.arg(
'--limit',
@ -681,14 +687,14 @@ def do_access_list(cs, args):
def do_list(cs, args):
"""List NAS shares with filters."""
list_of_keys = [
'ID', 'Name', 'Size', 'Share Proto', 'Status', 'Volume Type',
'ID', 'Name', 'Size', 'Share Proto', 'Status', 'Share Type',
'Export location', 'Host',
]
all_tenants = int(os.environ.get("ALL_TENANTS", args.all_tenants))
empty_obj = type('Empty', (object,), {'id': None})
volume_type = (_find_volume_type(cs, args.volume_type)
if args.volume_type else empty_obj)
share_type = (_find_share_type(cs, args.share_type)
if args.share_type else empty_obj)
snapshot = (_find_share_snapshot(cs, args.snapshot)
if args.snapshot else empty_obj)
@ -704,7 +710,7 @@ def do_list(cs, args):
'host': args.host,
'share_network_id': share_network.id,
'snapshot_id': snapshot.id,
'volume_type_id': volume_type.id,
'share_type_id': share_type.id,
'metadata': _extract_metadata(args),
'extra_specs': _extract_extra_specs(args),
'share_server_id': args.share_server_id,
@ -1576,67 +1582,67 @@ def do_service_list(cs, args):
cliutils.print_list(services, fields=fields)
def _print_type_extra_specs(vol_type):
def _print_type_extra_specs(share_type):
try:
return vol_type.get_keys()
return share_type.get_keys()
except exceptions.NotFound:
return "N/A"
return None
def _print_volume_type_list(vtypes):
cliutils.print_list(vtypes, ['ID', 'Name'])
def _print_share_type_list(stypes):
cliutils.print_list(stypes, ['ID', 'Name'])
def _print_type_and_extra_specs_list(vtypes):
def _print_type_and_extra_specs_list(stypes):
formatters = {'extra_specs': _print_type_extra_specs}
cliutils.print_list(vtypes, ['ID', 'Name', 'extra_specs'], formatters)
cliutils.print_list(stypes, ['ID', 'Name', 'extra_specs'], formatters)
def _find_volume_type(cs, vtype):
"""Get a volume type by name or ID."""
return apiclient_utils.find_resource(cs.volume_types, vtype)
def _find_share_type(cs, stype):
"""Get a share type by name or ID."""
return apiclient_utils.find_resource(cs.share_types, stype)
@cliutils.service_type('share')
def do_type_list(cs, args):
"""Print a list of available 'volume types'."""
vtypes = cs.volume_types.list()
_print_volume_type_list(vtypes)
"""Print a list of available 'share types'."""
stypes = cs.share_types.list()
_print_share_type_list(stypes)
@cliutils.service_type('share')
def do_extra_specs_list(cs, args):
"""Print a list of current 'volume types and extra specs' (Admin Only)."""
vtypes = cs.volume_types.list()
_print_type_and_extra_specs_list(vtypes)
"""Print a list of current 'share types and extra specs' (Admin Only)."""
stypes = cs.share_types.list()
_print_type_and_extra_specs_list(stypes)
@cliutils.arg(
'name',
metavar='<name>',
help="Name of the new volume type.")
help="Name of the new share type.")
@cliutils.service_type('share')
def do_type_create(cs, args):
"""Create a new volume type."""
vtype = cs.volume_types.create(args.name)
_print_volume_type_list([vtype])
"""Create a new share type."""
stype = cs.share_types.create(args.name)
_print_share_type_list([stype])
@cliutils.arg(
'id',
metavar='<id>',
help="Name or ID of the volume type to delete.")
help="Name or ID of the share type to delete.")
@cliutils.service_type('share')
def do_type_delete(cs, args):
"""Delete a specific volume type."""
volume_type = _find_volume_type(cs, args.id)
cs.volume_types.delete(volume_type)
"""Delete a specific share type."""
share_type = _find_share_type(cs, args.id)
cs.share_types.delete(share_type)
@cliutils.arg(
'vtype',
metavar='<vtype>',
help="Name or ID of the volume type.")
'stype',
metavar='<stype>',
help="Name or ID of the share type.")
@cliutils.arg(
'action',
metavar='<action>',
@ -1650,13 +1656,13 @@ def do_type_delete(cs, args):
help='Extra_specs to set or unset (key is only necessary on unset).')
@cliutils.service_type('share')
def do_type_key(cs, args):
"""Set or unset extra_spec for a volume type."""
vtype = _find_volume_type(cs, args.vtype)
"""Set or unset extra_spec for a share type."""
stype = _find_share_type(cs, args.stype)
if args.metadata is not None:
keypair = _extract_metadata(args)
if args.action == 'set':
vtype.set_keys(keypair)
stype.set_keys(keypair)
elif args.action == 'unset':
vtype.unset_keys(list(keypair))
stype.unset_keys(list(keypair))