From ea9ec1c6bc00ee90b288808f6d48d2ed420cf4b5 Mon Sep 17 00:00:00 2001 From: Dean Troyer Date: Thu, 11 Apr 2013 16:45:34 -0500 Subject: [PATCH] Tweak volume commands and add k=v argparse action Basic cleanups: * change metadata to property * add new KeyValueAction to parse the property options * multiple properties can be set using multiple --property args * consistent formatting * do lookups for volume args Change-Id: Ib6c43f01ad46b395aee8c61e886f42e2a5f5573e --- openstackclient/common/parseractions.py | 34 ++++++++ openstackclient/volume/v1/type.py | 36 +++++--- openstackclient/volume/v1/volume.py | 107 ++++++++++++++---------- tests/common/test_parseractions.py | 104 +++++++++++++++++++++++ 4 files changed, 223 insertions(+), 58 deletions(-) create mode 100644 openstackclient/common/parseractions.py create mode 100644 tests/common/test_parseractions.py diff --git a/openstackclient/common/parseractions.py b/openstackclient/common/parseractions.py new file mode 100644 index 000000000..f111c2642 --- /dev/null +++ b/openstackclient/common/parseractions.py @@ -0,0 +1,34 @@ +# Copyright 2013 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +"""argparse Custom Actions""" + +import argparse + + +class KeyValueAction(argparse.Action): + """A custom action to parse arguments as key=value pairs. + Ensures that dest is a dict + """ + def __call__(self, parser, namespace, values, option_string=None): + # Make sure we have an empty dict rather than None + if getattr(namespace, self.dest, None) is None: + setattr(namespace, self.dest, {}) + + # Add value if an assignment else remove it + if '=' in values: + getattr(namespace, self.dest, {}).update([values.split('=', 1)]) + else: + getattr(namespace, self.dest, {}).pop(values, None) diff --git a/openstackclient/volume/v1/type.py b/openstackclient/volume/v1/type.py index 9d79f8b3e..e146ee3f6 100644 --- a/openstackclient/volume/v1/type.py +++ b/openstackclient/volume/v1/type.py @@ -21,6 +21,7 @@ from cliff import command from cliff import lister from cliff import show +from openstackclient.common import parseractions from openstackclient.common import utils @@ -118,21 +119,22 @@ class SetVolumeType(command.Command): help='Volume type name or ID to update', ) parser.add_argument( - 'meta_data', + '--property', metavar='', - help='meta-data to add to volume type', + action=parseractions.KeyValueAction, + help='Property to add/change for this volume type ' + '(repeat option to set multiple properties)', ) return parser def take_action(self, parsed_args): self.log.debug('take_action(%s)' % parsed_args) - - meta = dict(v.split('=') for v in parsed_args.meta_data.split(' ')) volume_client = self.app.client_manager.volume volume_type = utils.find_resource( volume_client.volume_types, parsed_args.volume_type) - volume_type.set_keys(meta) + if parsed_args.property: + volume_type.set_keys(parsed_args.property) return @@ -148,12 +150,15 @@ class UnsetVolumeType(command.Command): parser.add_argument( 'volume_type', metavar='', - help='Type ID or name to update', + help='Type ID or name to remove', ) parser.add_argument( - 'meta_data', + '--property', metavar='', - help='meta-data to remove from volume type (key only)', + action='append', + default=[], + help='Property key to remove from volume ' + '(repeat option to remove multiple properties)', ) return parser @@ -161,12 +166,17 @@ class UnsetVolumeType(command.Command): self.log.debug('take_action(%s)' % parsed_args) volume_client = self.app.client_manager.volume volume_type = utils.find_resource( - volume_client.volume_types, parsed_args.volume_type) - - key_list = [] - key_list.append(parsed_args.meta_data) - volume_type.unset_keys(key_list) + volume_client.volume_types, + parsed_args.volume_type, + ) + if parsed_args.property: + volume_client.volumes.delete_metadata( + volume_type.id, + parsed_args.property, + ) + else: + self.app.log.error("No changes requested\n") return diff --git a/openstackclient/volume/v1/volume.py b/openstackclient/volume/v1/volume.py index f9641c548..43253c409 100644 --- a/openstackclient/volume/v1/volume.py +++ b/openstackclient/volume/v1/volume.py @@ -16,12 +16,12 @@ """Volume v1 Volume action implementations""" import logging -import sys from cliff import command from cliff import lister from cliff import show +from openstackclient.common import parseractions from openstackclient.common import utils @@ -63,32 +63,34 @@ class CreateVolume(show.ShowOne): parser.add_argument( '--user-id', metavar='', - help='User id derived from context', + help='Override user id derived from context (admin only)', ) parser.add_argument( '--project-id', metavar='', - help='Project id derived from context', + help='Override project id derived from context (admin only)', ) parser.add_argument( '--availability-zone', metavar='', - help='Availability Zone to use', + help='Availability zone to use', ) parser.add_argument( '--property', metavar='', - help='Optional property to set on volume creation', + action=parseractions.KeyValueAction, + help='Property to store for this volume ' + '(repeat option to set multiple properties)', ) parser.add_argument( - '--image-ref', - metavar='', - help='reference to an image stored in glance', + '--image', + metavar='', + help='Reference to a stored image', ) parser.add_argument( - '--source-volid', - metavar='', - help='ID of source volume to clone from', + '--source', + metavar='', + help='Source for volume clone', ) return parser @@ -98,22 +100,25 @@ class CreateVolume(show.ShowOne): volume_client = self.app.client_manager.volume - meta = None - if parsed_args.meta_data: - meta = dict(v.split('=') for v in parsed_args.meta_data.split(' ')) + source_volume = None + if parsed_args.source: + source_volume = utils.find_resource( + volume_client.volumes, + parsed_args.source, + ).id volume = volume_client.volumes.create( parsed_args.size, parsed_args.snapshot_id, - parsed_args.source_volid, + source_volume, parsed_args.name, parsed_args.description, parsed_args.volume_type, parsed_args.user_id, parsed_args.project_id, parsed_args.availability_zone, - meta, - parsed_args.image_ref + parsed_args.property, + parsed_args.image ) return zip(*sorted(volume._info.iteritems())) @@ -175,13 +180,13 @@ class ListVolume(lister.Lister): '--all-tenants', action='store_true', default=False, - help='Display information from all tenants (Admin-only)', + help='Display information from all tenants (admin only)', ) parser.add_argument( '--long', action='store_true', default=False, - help='Display meta-data', + help='Display properties', ) return parser @@ -221,19 +226,25 @@ class SetVolume(command.Command): parser.add_argument( 'volume', metavar='', - help='Name or ID of volume to change') + help='Name or ID of volume to change', + ) parser.add_argument( '--name', - metavar='', - help='New volume name') + metavar='', + help='New volume name', + ) parser.add_argument( '--description', - metavar='', - help='New volume description') + metavar='', + help='New volume description', + ) parser.add_argument( - '--meta-data', + '--property', metavar='', - help='meta-data to add to volume') + action=parseractions.KeyValueAction, + help='Property to add/change for this volume ' + '(repeat option to set multiple properties)', + ) return parser def take_action(self, parsed_args): @@ -241,21 +252,22 @@ class SetVolume(command.Command): volume_client = self.app.client_manager.volume volume = utils.find_resource(volume_client.volumes, parsed_args.volume) - meta = None if parsed_args.property: - meta = dict(v.split('=') for v in parsed_args.property.split(' ')) - volume_client.volumes.set_metadata(volume.id, meta) + print "property: %s" % parsed_args.property + volume_client.volumes.set_metadata(volume.id, parsed_args.property) kwargs = {} if parsed_args.name: kwargs['display_name'] = parsed_args.name if parsed_args.description: kwargs['display_description'] = parsed_args.description + if kwargs: + print "kwargs: %s" % kwargs + volume_client.volumes.update(volume.id, **kwargs) + + if not kwargs and not parsed_args.property: + self.app.log.error("No changes requested\n") - if not kwargs and not meta: - sys.stdout.write("Volume not updated, no arguments present \n") - return - volume_client.volumes.update(volume.id, **kwargs) return @@ -270,7 +282,8 @@ class ShowVolume(show.ShowOne): parser.add_argument( 'volume', metavar='', - help='Name or ID of volume to display') + help='Name or ID of volume to display', + ) return parser def take_action(self, parsed_args): @@ -292,11 +305,16 @@ class UnsetVolume(command.Command): parser.add_argument( 'volume', metavar='', - help='Name or ID of volume to change') + help='Name or ID of volume to change', + ) parser.add_argument( - '--meta-data', + '--property', metavar='', - help='meta-data to remove from volume (key only)') + action='append', + default=[], + help='Property key to remove from volume ' + '(repeat to set multiple values)', + ) return parser def take_action(self, parsed_args): @@ -305,14 +323,13 @@ class UnsetVolume(command.Command): volume = utils.find_resource( volume_client.volumes, parsed_args.volume) - if not parsed_args.meta_data: - sys.stdout.write("Volume not updated, no arguments present \n") - return - - key_list = [] - key_list.append(parsed_args.meta_data) - volume_client.volumes.delete_metadata(volume.id, key_list) - + if parsed_args.property: + volume_client.volumes.delete_metadata( + volume.id, + parsed_args.property, + ) + else: + self.app.log.error("No changes requested\n") return diff --git a/tests/common/test_parseractions.py b/tests/common/test_parseractions.py new file mode 100644 index 000000000..f48c4d721 --- /dev/null +++ b/tests/common/test_parseractions.py @@ -0,0 +1,104 @@ +# Copyright 2012-2013 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +import argparse + +from openstackclient.common import parseractions +from tests import utils + + +class TestKeyValueAction(utils.TestCase): + def test_good_values(self): + parser = argparse.ArgumentParser() + + # Set up our typical usage + parser.add_argument( + '--property', + metavar='', + action=parseractions.KeyValueAction, + help='Property to store for this volume ' + '(repeat option to set multiple properties)', + ) + + results = parser.parse_args([ + '--property', 'red=', + '--property', 'green=100%', + '--property', 'blue=50%', + ]) + + actual = getattr(results, 'property', {}) + # All should pass through unmolested + expect = {'red': '', 'green': '100%', 'blue': '50%'} + self.assertDictEqual(expect, actual) + + def test_default_values(self): + parser = argparse.ArgumentParser() + + # Set up our typical usage + parser.add_argument( + '--property', + metavar='', + action=parseractions.KeyValueAction, + default={'green': '20%', 'format': '#rgb'}, + help='Property to store for this volume ' + '(repeat option to set multiple properties)', + ) + + results = parser.parse_args([ + '--property', 'red=', + '--property', 'green=100%', + '--property', 'blue=50%', + ]) + + actual = getattr(results, 'property', {}) + # Verify green default is changed, format default is unchanged + expect = {'red': '', 'green': '100%', 'blue': '50%', 'format': '#rgb'} + self.assertDictEqual(expect, actual) + + def test_error_values(self): + parser = argparse.ArgumentParser() + + # Set up our typical usage + parser.add_argument( + '--property', + metavar='', + action=parseractions.KeyValueAction, + default={'green': '20%', 'blue': '40%'}, + help='Property to store for this volume ' + '(repeat option to set multiple properties)', + ) + + results = parser.parse_args([ + '--property', 'red', + '--property', 'green=100%', + '--property', 'blue', + ]) + + failhere = None + actual = getattr(results, 'property', {}) + # Verify non-existant red key + try: + failhere = actual['red'] + except Exception as e: + self.assertTrue(type(e) == KeyError) + # Verify removal of blue key + try: + failhere = actual['blue'] + except Exception as e: + self.assertTrue(type(e) == KeyError) + # There should be no red or blue + expect = {'green': '100%'} + self.assertDictEqual(expect, actual) + self.assertEqual(failhere, None)