From f4ddf3626c50841308abe30ae5c065c75830b476 Mon Sep 17 00:00:00 2001 From: Bob Callaway Date: Sun, 15 Feb 2015 14:59:07 -0800 Subject: [PATCH] Rename volume_type to share_type 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 --- manilaclient/common/constants.py | 2 +- ...st_volume_types.py => test_share_types.py} | 6 +- manilaclient/tests/unit/v1/fakes.py | 18 ++-- manilaclient/tests/unit/v1/test_shares.py | 6 +- manilaclient/tests/unit/v1/test_shell.py | 26 ++--- manilaclient/tests/unit/v1/test_types.py | 16 +-- manilaclient/v1/client.py | 4 +- .../v1/{volume_types.py => share_types.py} | 62 ++++++------ manilaclient/v1/shares.py | 16 +-- manilaclient/v1/shell.py | 98 ++++++++++--------- 10 files changed, 130 insertions(+), 124 deletions(-) rename manilaclient/tests/functional/{test_volume_types.py => test_share_types.py} (86%) rename manilaclient/v1/{volume_types.py => share_types.py} (57%) diff --git a/manilaclient/common/constants.py b/manilaclient/common/constants.py index 2ac268ccc..71bb4846f 100644 --- a/manilaclient/common/constants.py +++ b/manilaclient/common/constants.py @@ -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', ) diff --git a/manilaclient/tests/functional/test_volume_types.py b/manilaclient/tests/functional/test_share_types.py similarity index 86% rename from manilaclient/tests/functional/test_volume_types.py rename to manilaclient/tests/functional/test_share_types.py index f7db45eba..4bf446415 100644 --- a/manilaclient/tests/functional/test_volume_types.py +++ b/manilaclient/tests/functional/test_share_types.py @@ -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') diff --git a/manilaclient/tests/unit/v1/fakes.py b/manilaclient/tests/unit/v1/fakes.py index 09e04e690..e6b4a1563 100644 --- a/manilaclient/tests/unit/v1/fakes.py +++ b/manilaclient/tests/unit/v1/fakes.py @@ -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): diff --git a/manilaclient/tests/unit/v1/test_shares.py b/manilaclient/tests/unit/v1/test_shares.py index 2c683831f..2988db62c 100644 --- a/manilaclient/tests/unit/v1/test_shares.py +++ b/manilaclient/tests/unit/v1/test_shares.py @@ -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') diff --git a/manilaclient/tests/unit/v1/test_shell.py b/manilaclient/tests/unit/v1/test_shell.py index 50572550b..02e467f4a 100644 --- a/manilaclient/tests/unit/v1/test_shell.py +++ b/manilaclient/tests/unit/v1/test_shell.py @@ -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, diff --git a/manilaclient/tests/unit/v1/test_types.py b/manilaclient/tests/unit/v1/test_types.py index 44ca63029..93c2ca8f5 100644 --- a/manilaclient/tests/unit/v1/test_types.py +++ b/manilaclient/tests/unit/v1/test_types.py @@ -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') diff --git a/manilaclient/v1/client.py b/manilaclient/v1/client.py index 6c977d0fd..edf0c44e6 100644 --- a/manilaclient/v1/client.py +++ b/manilaclient/v1/client.py @@ -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... diff --git a/manilaclient/v1/volume_types.py b/manilaclient/v1/share_types.py similarity index 57% rename from manilaclient/v1/volume_types.py rename to manilaclient/v1/share_types.py index 0208a6973..5ba1aaa9f 100644 --- a/manilaclient/v1/volume_types.py +++ b/manilaclient/v1/share_types.py @@ -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 "" % self.name + return "" % 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") diff --git a/manilaclient/v1/shares.py b/manilaclient/v1/shares.py index a3e473035..9898c0696 100644 --- a/manilaclient/v1/shares.py +++ b/manilaclient/v1/shares.py @@ -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} diff --git a/manilaclient/v1/shell.py b/manilaclient/v1/shell.py index 3ce7c8ad9..999cd5051 100644 --- a/manilaclient/v1/shell.py +++ b/manilaclient/v1/shell.py @@ -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='', - help='Optional volume type. (Default=None)', - default=None) + '--share-type', '--share_type', '--volume-type', '--volume_type', + metavar='', + 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='', 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='', + '--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='', 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='', - 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='', - 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='', - help="Name or ID of the volume type.") + 'stype', + metavar='', + help="Name or ID of the share type.") @cliutils.arg( 'action', metavar='', @@ -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))