From 80eaa33ffe8ad2f5e931341d564f538a5a94618e Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 18 Jul 2024 13:24:04 +0100 Subject: [PATCH] volume: Make better use of argparse The latest in a series. Change-Id: I8273c817e38120ba0b25aebdbfa1c2872222765e Signed-off-by: Stephen Finucane --- .../tests/unit/volume/v2/test_volume.py | 92 +++++++++--------- .../tests/unit/volume/v3/test_volume.py | 93 +++++++++---------- openstackclient/volume/v2/volume.py | 71 +++++++++----- openstackclient/volume/v3/volume.py | 63 ++++++++----- 4 files changed, 178 insertions(+), 141 deletions(-) diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py index b293ee57da..5df96f27d4 100644 --- a/openstackclient/tests/unit/volume/v2/test_volume.py +++ b/openstackclient/tests/unit/volume/v2/test_volume.py @@ -188,7 +188,7 @@ class TestVolumeCreate(TestVolume): self.new_volume.name, ] verifylist = [ - ('property', {'Alpha': 'a', 'Beta': 'b'}), + ('properties', {'Alpha': 'a', 'Beta': 'b'}), ('size', self.new_volume.size), ('name', self.new_volume.name), ] @@ -382,9 +382,7 @@ class TestVolumeCreate(TestVolume): ] verifylist = [ ('bootable', True), - ('non_bootable', False), ('read_only', True), - ('read_write', False), ('size', self.new_volume.size), ('name', self.new_volume.name), ] @@ -427,9 +425,7 @@ class TestVolumeCreate(TestVolume): ] verifylist = [ ('bootable', False), - ('non_bootable', True), ('read_only', False), - ('read_write', True), ('size', self.new_volume.size), ('name', self.new_volume.name), ] @@ -481,9 +477,7 @@ class TestVolumeCreate(TestVolume): ] verifylist = [ ('bootable', True), - ('non_bootable', False), ('read_only', True), - ('read_write', False), ('size', self.new_volume.size), ('name', self.new_volume.name), ] @@ -532,9 +526,7 @@ class TestVolumeCreate(TestVolume): ] verifylist = [ ('bootable', False), - ('non_bootable', True), ('read_only', True), - ('read_write', False), ('size', self.new_volume.size), ('name', self.new_volume.name), ] @@ -1407,16 +1399,16 @@ class TestVolumeSet(TestVolume): self.new_volume.id, ] verifylist = [ - ('property', {'a': 'b', 'c': 'd'}), + ('properties', {'a': 'b', 'c': 'd'}), ('volume', self.new_volume.id), - ('bootable', False), - ('non_bootable', False), + ('bootable', None), + ('read_only', None), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.cmd.take_action(parsed_args) self.volumes_mock.set_metadata.assert_called_with( - self.new_volume.id, parsed_args.property + self.new_volume.id, parsed_args.properties ) def test_volume_set_image_property(self): @@ -1428,10 +1420,10 @@ class TestVolumeSet(TestVolume): self.new_volume.id, ] verifylist = [ - ('image_property', {'Alpha': 'a', 'Beta': 'b'}), + ('image_properties', {'Alpha': 'a', 'Beta': 'b'}), ('volume', self.new_volume.id), - ('bootable', False), - ('non_bootable', False), + ('bootable', None), + ('read_only', None), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1439,14 +1431,13 @@ class TestVolumeSet(TestVolume): # returns nothing self.cmd.take_action(parsed_args) self.volumes_mock.set_image_metadata.assert_called_with( - self.new_volume.id, parsed_args.image_property + self.new_volume.id, parsed_args.image_properties ) def test_volume_set_state(self): arglist = ['--state', 'error', self.new_volume.id] verifylist = [ - ('read_only', False), - ('read_write', False), + ('read_only', None), ('state', 'error'), ('volume', self.new_volume.id), ] @@ -1511,36 +1502,40 @@ class TestVolumeSet(TestVolume): def test_volume_set_bootable(self): arglist = [ - ['--bootable', self.new_volume.id], - ['--non-bootable', self.new_volume.id], + '--bootable', + self.new_volume.id, ] verifylist = [ - [ - ('bootable', True), - ('non_bootable', False), - ('volume', self.new_volume.id), - ], - [ - ('bootable', False), - ('non_bootable', True), - ('volume', self.new_volume.id), - ], + ('bootable', True), + ('volume', self.new_volume.id), ] - for index in range(len(arglist)): - parsed_args = self.check_parser( - self.cmd, arglist[index], verifylist[index] - ) + parsed_args = self.check_parser(self.cmd, arglist, verifylist) - self.cmd.take_action(parsed_args) - self.volumes_mock.set_bootable.assert_called_with( - self.new_volume.id, verifylist[index][0][1] - ) + self.cmd.take_action(parsed_args) + self.volumes_mock.set_bootable.assert_called_with( + self.new_volume.id, verifylist[0][1] + ) - def test_volume_set_readonly(self): + def test_volume_set_non_bootable(self): + arglist = [ + '--non-bootable', + self.new_volume.id, + ] + verifylist = [ + ('bootable', False), + ('volume', self.new_volume.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.cmd.take_action(parsed_args) + self.volumes_mock.set_bootable.assert_called_with( + self.new_volume.id, verifylist[0][1] + ) + + def test_volume_set_read_only(self): arglist = ['--read-only', self.new_volume.id] verifylist = [ ('read_only', True), - ('read_write', False), ('volume', self.new_volume.id), ] @@ -1556,7 +1551,6 @@ class TestVolumeSet(TestVolume): arglist = ['--read-write', self.new_volume.id] verifylist = [ ('read_only', False), - ('read_write', True), ('volume', self.new_volume.id), ] @@ -1686,7 +1680,7 @@ class TestVolumeUnset(TestVolume): self.new_volume.id, ] verifylist = [ - ('image_property', {'Alpha': 'a', 'Beta': 'b'}), + ('image_properties', {'Alpha': 'a', 'Beta': 'b'}), ('volume', self.new_volume.id), ] parsed_args = self.check_parser(self.cmd_set, arglist, verifylist) @@ -1702,7 +1696,7 @@ class TestVolumeUnset(TestVolume): self.new_volume.id, ] verifylist_unset = [ - ('image_property', ['Alpha']), + ('image_properties', ['Alpha']), ('volume', self.new_volume.id), ] parsed_args_unset = self.check_parser( @@ -1714,7 +1708,7 @@ class TestVolumeUnset(TestVolume): self.cmd_unset.take_action(parsed_args_unset) self.volumes_mock.delete_image_metadata.assert_called_with( - self.new_volume.id, parsed_args_unset.image_property + self.new_volume.id, parsed_args_unset.image_properties ) def test_volume_unset_image_property_fail(self): @@ -1729,8 +1723,8 @@ class TestVolumeUnset(TestVolume): self.new_volume.id, ] verifylist = [ - ('image_property', ['Alpha']), - ('property', ['Beta']), + ('image_properties', ['Alpha']), + ('properties', ['Beta']), ('volume', self.new_volume.id), ] parsed_args = self.check_parser(self.cmd_unset, arglist, verifylist) @@ -1743,10 +1737,10 @@ class TestVolumeUnset(TestVolume): 'One or more of the unset operations failed', str(e) ) self.volumes_mock.delete_image_metadata.assert_called_with( - self.new_volume.id, parsed_args.image_property + self.new_volume.id, parsed_args.image_properties ) self.volumes_mock.delete_metadata.assert_called_with( - self.new_volume.id, parsed_args.property + self.new_volume.id, parsed_args.properties ) diff --git a/openstackclient/tests/unit/volume/v3/test_volume.py b/openstackclient/tests/unit/volume/v3/test_volume.py index e3dc016656..6bf43d73f4 100644 --- a/openstackclient/tests/unit/volume/v3/test_volume.py +++ b/openstackclient/tests/unit/volume/v3/test_volume.py @@ -179,7 +179,7 @@ class TestVolumeCreateLegacy(volume_fakes.TestVolume): self.new_volume.name, ] verifylist = [ - ('property', {'Alpha': 'a', 'Beta': 'b'}), + ('properties', {'Alpha': 'a', 'Beta': 'b'}), ('size', self.new_volume.size), ('name', self.new_volume.name), ] @@ -440,9 +440,7 @@ class TestVolumeCreateLegacy(volume_fakes.TestVolume): ] verifylist = [ ('bootable', True), - ('non_bootable', False), ('read_only', True), - ('read_write', False), ('size', self.new_volume.size), ('name', self.new_volume.name), ] @@ -486,9 +484,7 @@ class TestVolumeCreateLegacy(volume_fakes.TestVolume): ] verifylist = [ ('bootable', False), - ('non_bootable', True), ('read_only', False), - ('read_write', True), ('size', self.new_volume.size), ('name', self.new_volume.name), ] @@ -541,9 +537,7 @@ class TestVolumeCreateLegacy(volume_fakes.TestVolume): ] verifylist = [ ('bootable', True), - ('non_bootable', False), ('read_only', True), - ('read_write', False), ('size', self.new_volume.size), ('name', self.new_volume.name), ] @@ -593,9 +587,7 @@ class TestVolumeCreateLegacy(volume_fakes.TestVolume): ] verifylist = [ ('bootable', False), - ('non_bootable', True), ('read_only', True), - ('read_write', False), ('size', self.new_volume.size), ('name', self.new_volume.name), ] @@ -838,7 +830,7 @@ class TestVolumeCreate(volume_fakes.TestVolume): description=parsed_args.description, volume_type=parsed_args.type, availability_zone=parsed_args.availability_zone, - metadata=parsed_args.property, + metadata=parsed_args.properties, bootable=parsed_args.bootable, cluster=getattr(parsed_args, 'cluster', None), ) @@ -1815,16 +1807,16 @@ class TestVolumeSet(volume_fakes.TestVolume): self.new_volume.id, ] verifylist = [ - ('property', {'a': 'b', 'c': 'd'}), + ('properties', {'a': 'b', 'c': 'd'}), + ('read_only', None), + ('bootable', None), ('volume', self.new_volume.id), - ('bootable', False), - ('non_bootable', False), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.cmd.take_action(parsed_args) self.volumes_mock.set_metadata.assert_called_with( - self.new_volume.id, parsed_args.property + self.new_volume.id, parsed_args.properties ) def test_volume_set_image_property(self): @@ -1836,10 +1828,10 @@ class TestVolumeSet(volume_fakes.TestVolume): self.new_volume.id, ] verifylist = [ - ('image_property', {'Alpha': 'a', 'Beta': 'b'}), + ('image_properties', {'Alpha': 'a', 'Beta': 'b'}), + ('read_only', None), + ('bootable', None), ('volume', self.new_volume.id), - ('bootable', False), - ('non_bootable', False), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -1847,15 +1839,15 @@ class TestVolumeSet(volume_fakes.TestVolume): # returns nothing self.cmd.take_action(parsed_args) self.volumes_mock.set_image_metadata.assert_called_with( - self.new_volume.id, parsed_args.image_property + self.new_volume.id, parsed_args.image_properties ) def test_volume_set_state(self): arglist = ['--state', 'error', self.new_volume.id] verifylist = [ - ('read_only', False), - ('read_write', False), ('state', 'error'), + ('read_only', None), + ('bootable', None), ('volume', self.new_volume.id), ] @@ -1919,36 +1911,40 @@ class TestVolumeSet(volume_fakes.TestVolume): def test_volume_set_bootable(self): arglist = [ - ['--bootable', self.new_volume.id], - ['--non-bootable', self.new_volume.id], + '--bootable', + self.new_volume.id, ] verifylist = [ - [ - ('bootable', True), - ('non_bootable', False), - ('volume', self.new_volume.id), - ], - [ - ('bootable', False), - ('non_bootable', True), - ('volume', self.new_volume.id), - ], + ('bootable', True), + ('volume', self.new_volume.id), ] - for index in range(len(arglist)): - parsed_args = self.check_parser( - self.cmd, arglist[index], verifylist[index] - ) + parsed_args = self.check_parser(self.cmd, arglist, verifylist) - self.cmd.take_action(parsed_args) - self.volumes_mock.set_bootable.assert_called_with( - self.new_volume.id, verifylist[index][0][1] - ) + self.cmd.take_action(parsed_args) + self.volumes_mock.set_bootable.assert_called_with( + self.new_volume.id, verifylist[0][1] + ) + + def test_volume_set_non_bootable(self): + arglist = [ + '--non-bootable', + self.new_volume.id, + ] + verifylist = [ + ('bootable', False), + ('volume', self.new_volume.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.cmd.take_action(parsed_args) + self.volumes_mock.set_bootable.assert_called_with( + self.new_volume.id, verifylist[0][1] + ) def test_volume_set_readonly(self): arglist = ['--read-only', self.new_volume.id] verifylist = [ ('read_only', True), - ('read_write', False), ('volume', self.new_volume.id), ] @@ -1964,7 +1960,6 @@ class TestVolumeSet(volume_fakes.TestVolume): arglist = ['--read-write', self.new_volume.id] verifylist = [ ('read_only', False), - ('read_write', True), ('volume', self.new_volume.id), ] @@ -2100,7 +2095,7 @@ class TestVolumeUnset(volume_fakes.TestVolume): self.new_volume.id, ] verifylist = [ - ('image_property', {'Alpha': 'a', 'Beta': 'b'}), + ('image_properties', {'Alpha': 'a', 'Beta': 'b'}), ('volume', self.new_volume.id), ] parsed_args = self.check_parser(self.cmd_set, arglist, verifylist) @@ -2116,7 +2111,7 @@ class TestVolumeUnset(volume_fakes.TestVolume): self.new_volume.id, ] verifylist_unset = [ - ('image_property', ['Alpha']), + ('image_properties', ['Alpha']), ('volume', self.new_volume.id), ] parsed_args_unset = self.check_parser( @@ -2128,7 +2123,7 @@ class TestVolumeUnset(volume_fakes.TestVolume): self.cmd_unset.take_action(parsed_args_unset) self.volumes_mock.delete_image_metadata.assert_called_with( - self.new_volume.id, parsed_args_unset.image_property + self.new_volume.id, parsed_args_unset.image_properties ) def test_volume_unset_image_property_fail(self): @@ -2143,8 +2138,8 @@ class TestVolumeUnset(volume_fakes.TestVolume): self.new_volume.id, ] verifylist = [ - ('image_property', ['Alpha']), - ('property', ['Beta']), + ('image_properties', ['Alpha']), + ('properties', ['Beta']), ('volume', self.new_volume.id), ] parsed_args = self.check_parser(self.cmd_unset, arglist, verifylist) @@ -2157,10 +2152,10 @@ class TestVolumeUnset(volume_fakes.TestVolume): 'One or more of the unset operations failed', str(e) ) self.volumes_mock.delete_image_metadata.assert_called_with( - self.new_volume.id, parsed_args.image_property + self.new_volume.id, parsed_args.image_properties ) self.volumes_mock.delete_metadata.assert_called_with( - self.new_volume.id, parsed_args.property + self.new_volume.id, parsed_args.properties ) diff --git a/openstackclient/volume/v2/volume.py b/openstackclient/volume/v2/volume.py index 8b55ce2a8d..193e82f013 100644 --- a/openstackclient/volume/v2/volume.py +++ b/openstackclient/volume/v2/volume.py @@ -169,6 +169,7 @@ class CreateVolume(command.ShowOne): "--property", metavar="", action=parseractions.KeyValueAction, + dest="properties", help=_( "Set a property to this volume " "(repeat option to set multiple properties)" @@ -189,22 +190,30 @@ class CreateVolume(command.ShowOne): bootable_group.add_argument( "--bootable", action="store_true", + dest="bootable", + default=None, help=_("Mark volume as bootable"), ) bootable_group.add_argument( "--non-bootable", - action="store_true", + action="store_false", + dest="bootable", + default=None, help=_("Mark volume as non-bootable (default)"), ) readonly_group = parser.add_mutually_exclusive_group() readonly_group.add_argument( "--read-only", action="store_true", + dest="read_only", + default=None, help=_("Set volume to read-only access mode"), ) readonly_group.add_argument( "--read-write", - action="store_true", + action="store_false", + dest="read_only", + default=None, help=_("Set volume to read-write access mode (default)"), ) return parser, source_group @@ -265,14 +274,14 @@ class CreateVolume(command.ShowOne): description=parsed_args.description, volume_type=parsed_args.type, availability_zone=parsed_args.availability_zone, - metadata=parsed_args.property, + metadata=parsed_args.properties, imageRef=image, source_volid=source_volume, consistencygroup_id=consistency_group, scheduler_hints=parsed_args.hint, ) - if parsed_args.bootable or parsed_args.non_bootable: + if parsed_args.bootable is not None: try: if utils.wait_for_status( volume_client.volumes.get, @@ -291,7 +300,8 @@ class CreateVolume(command.ShowOne): raise exceptions.CommandError(msg) except Exception as e: LOG.error(_("Failed to set volume bootable property: %s"), e) - if parsed_args.read_only or parsed_args.read_write: + + if parsed_args.read_only is not None: try: if utils.wait_for_status( volume_client.volumes.get, @@ -622,6 +632,7 @@ class SetVolume(command.Command): '--property', metavar='', action=parseractions.KeyValueAction, + dest="properties", help=_( 'Set a property on this volume ' '(repeat option to set multiple properties)' @@ -631,6 +642,7 @@ class SetVolume(command.Command): '--image-property', metavar='', action=parseractions.KeyValueAction, + dest="image_properties", help=_( 'Set an image property on this volume ' '(repeat option to set multiple image properties)' @@ -707,22 +719,30 @@ class SetVolume(command.Command): bootable_group.add_argument( "--bootable", action="store_true", + dest="bootable", + default=None, help=_("Mark volume as bootable"), ) bootable_group.add_argument( "--non-bootable", - action="store_true", + action="store_false", + dest="bootable", + default=None, help=_("Mark volume as non-bootable"), ) readonly_group = parser.add_mutually_exclusive_group() readonly_group.add_argument( "--read-only", action="store_true", + dest="read_only", + default=None, help=_("Set volume to read-only access mode"), ) readonly_group.add_argument( "--read-write", - action="store_true", + action="store_false", + dest="read_only", + default=None, help=_("Set volume to read-write access mode"), ) return parser @@ -771,28 +791,31 @@ class SetVolume(command.Command): LOG.error(_("Failed to clean volume properties: %s"), e) result += 1 - if parsed_args.property: + if parsed_args.properties: try: volume_client.volumes.set_metadata( - volume.id, parsed_args.property + volume.id, parsed_args.properties ) except Exception as e: - LOG.error(_("Failed to set volume property: %s"), e) + LOG.error(_("Failed to set volume properties: %s"), e) result += 1 - if parsed_args.image_property: + + if parsed_args.image_properties: try: volume_client.volumes.set_image_metadata( - volume.id, parsed_args.image_property + volume.id, parsed_args.image_properties ) except Exception as e: - LOG.error(_("Failed to set image property: %s"), e) + LOG.error(_("Failed to set image properties: %s"), e) result += 1 + if parsed_args.state: try: volume_client.volumes.reset_state(volume.id, parsed_args.state) except Exception as e: LOG.error(_("Failed to set volume state: %s"), e) result += 1 + if parsed_args.attached: try: volume_client.volumes.reset_state( @@ -801,6 +824,7 @@ class SetVolume(command.Command): except Exception as e: LOG.error(_("Failed to set volume attach-status: %s"), e) result += 1 + if parsed_args.detached: try: volume_client.volumes.reset_state( @@ -809,7 +833,8 @@ class SetVolume(command.Command): except Exception as e: LOG.error(_("Failed to set volume attach-status: %s"), e) result += 1 - if parsed_args.bootable or parsed_args.non_bootable: + + if parsed_args.bootable is not None: try: volume_client.volumes.set_bootable( volume.id, parsed_args.bootable @@ -817,7 +842,8 @@ class SetVolume(command.Command): except Exception as e: LOG.error(_("Failed to set volume bootable property: %s"), e) result += 1 - if parsed_args.read_only or parsed_args.read_write: + + if parsed_args.read_only is not None: try: volume_client.volumes.update_readonly_flag( volume.id, parsed_args.read_only @@ -828,6 +854,7 @@ class SetVolume(command.Command): e, ) result += 1 + policy = parsed_args.migration_policy or parsed_args.retype_policy if parsed_args.type: # get the migration policy @@ -928,6 +955,7 @@ class UnsetVolume(command.Command): '--property', metavar='', action='append', + dest='properties', help=_( 'Remove a property from volume ' '(repeat option to remove multiple properties)' @@ -937,6 +965,7 @@ class UnsetVolume(command.Command): '--image-property', metavar='', action='append', + dest='image_properties', help=_( 'Remove an image property from volume ' '(repeat option to remove multiple image properties)' @@ -949,22 +978,22 @@ class UnsetVolume(command.Command): volume = utils.find_resource(volume_client.volumes, parsed_args.volume) result = 0 - if parsed_args.property: + if parsed_args.properties: try: volume_client.volumes.delete_metadata( - volume.id, parsed_args.property + volume.id, parsed_args.properties ) except Exception as e: - LOG.error(_("Failed to unset volume property: %s"), e) + LOG.error(_("Failed to unset volume properties: %s"), e) result += 1 - if parsed_args.image_property: + if parsed_args.image_properties: try: volume_client.volumes.delete_image_metadata( - volume.id, parsed_args.image_property + volume.id, parsed_args.image_properties ) except Exception as e: - LOG.error(_("Failed to unset image property: %s"), e) + LOG.error(_("Failed to unset image properties: %s"), e) result += 1 if result > 0: diff --git a/openstackclient/volume/v3/volume.py b/openstackclient/volume/v3/volume.py index 3a07a6534c..64aa5f076f 100644 --- a/openstackclient/volume/v3/volume.py +++ b/openstackclient/volume/v3/volume.py @@ -194,8 +194,7 @@ class CreateVolume(volume_v2.CreateVolume): parsed_args.size or parsed_args.consistency_group or parsed_args.hint - or parsed_args.read_only - or parsed_args.read_write + or parsed_args.read_only is not None ): msg = _( "The --size, --consistency-group, --hint, --read-only " @@ -232,7 +231,7 @@ class CreateVolume(volume_v2.CreateVolume): description=parsed_args.description, volume_type=parsed_args.type, availability_zone=parsed_args.availability_zone, - metadata=parsed_args.property, + metadata=parsed_args.properties, bootable=parsed_args.bootable, ) return zip(*sorted(volume.items())) @@ -287,7 +286,7 @@ class CreateVolume(volume_v2.CreateVolume): description=parsed_args.description, volume_type=parsed_args.type, availability_zone=parsed_args.availability_zone, - metadata=parsed_args.property, + metadata=parsed_args.properties, imageRef=image, source_volid=source_volume, consistencygroup_id=consistency_group, @@ -295,7 +294,7 @@ class CreateVolume(volume_v2.CreateVolume): backup_id=backup, ) - if parsed_args.bootable or parsed_args.non_bootable: + if parsed_args.bootable is not None: try: if utils.wait_for_status( volume_client.volumes.get, @@ -314,7 +313,8 @@ class CreateVolume(volume_v2.CreateVolume): raise exceptions.CommandError(msg) except Exception as e: LOG.error(_("Failed to set volume bootable property: %s"), e) - if parsed_args.read_only or parsed_args.read_write: + + if parsed_args.read_only is not None: try: if utils.wait_for_status( volume_client.volumes.get, @@ -638,6 +638,7 @@ class SetVolume(command.Command): '--property', metavar='', action=parseractions.KeyValueAction, + dest='properties', help=_( 'Set a property on this volume ' '(repeat option to set multiple properties)' @@ -647,6 +648,7 @@ class SetVolume(command.Command): '--image-property', metavar='', action=parseractions.KeyValueAction, + dest='image_properties', help=_( 'Set an image property on this volume ' '(repeat option to set multiple image properties)' @@ -723,22 +725,30 @@ class SetVolume(command.Command): bootable_group.add_argument( "--bootable", action="store_true", + dest="bootable", + default=None, help=_("Mark volume as bootable"), ) bootable_group.add_argument( "--non-bootable", - action="store_true", + action="store_false", + dest="bootable", + default=None, help=_("Mark volume as non-bootable"), ) readonly_group = parser.add_mutually_exclusive_group() readonly_group.add_argument( "--read-only", action="store_true", + dest="read_only", + default=None, help=_("Set volume to read-only access mode"), ) readonly_group.add_argument( "--read-write", - action="store_true", + action="store_false", + dest="read_only", + default=None, help=_("Set volume to read-write access mode"), ) return parser @@ -796,28 +806,31 @@ class SetVolume(command.Command): LOG.error(_("Failed to clean volume properties: %s"), e) result += 1 - if parsed_args.property: + if parsed_args.properties: try: volume_client.volumes.set_metadata( - volume.id, parsed_args.property + volume.id, parsed_args.properties ) except Exception as e: - LOG.error(_("Failed to set volume property: %s"), e) + LOG.error(_("Failed to set volume properties: %s"), e) result += 1 - if parsed_args.image_property: + + if parsed_args.image_properties: try: volume_client.volumes.set_image_metadata( - volume.id, parsed_args.image_property + volume.id, parsed_args.image_properties ) except Exception as e: - LOG.error(_("Failed to set image property: %s"), e) + LOG.error(_("Failed to set image properties: %s"), e) result += 1 + if parsed_args.state: try: volume_client.volumes.reset_state(volume.id, parsed_args.state) except Exception as e: LOG.error(_("Failed to set volume state: %s"), e) result += 1 + if parsed_args.attached: try: volume_client.volumes.reset_state( @@ -826,6 +839,7 @@ class SetVolume(command.Command): except Exception as e: LOG.error(_("Failed to set volume attach-status: %s"), e) result += 1 + if parsed_args.detached: try: volume_client.volumes.reset_state( @@ -834,7 +848,8 @@ class SetVolume(command.Command): except Exception as e: LOG.error(_("Failed to set volume attach-status: %s"), e) result += 1 - if parsed_args.bootable or parsed_args.non_bootable: + + if parsed_args.bootable is not None: try: volume_client.volumes.set_bootable( volume.id, parsed_args.bootable @@ -842,7 +857,8 @@ class SetVolume(command.Command): except Exception as e: LOG.error(_("Failed to set volume bootable property: %s"), e) result += 1 - if parsed_args.read_only or parsed_args.read_write: + + if parsed_args.read_only is not None: try: volume_client.volumes.update_readonly_flag( volume.id, parsed_args.read_only @@ -853,6 +869,7 @@ class SetVolume(command.Command): e, ) result += 1 + policy = parsed_args.migration_policy or parsed_args.retype_policy if parsed_args.type: # get the migration policy @@ -953,6 +970,7 @@ class UnsetVolume(command.Command): '--property', metavar='', action='append', + dest='properties', help=_( 'Remove a property from volume ' '(repeat option to remove multiple properties)' @@ -962,6 +980,7 @@ class UnsetVolume(command.Command): '--image-property', metavar='', action='append', + dest='image_properties', help=_( 'Remove an image property from volume ' '(repeat option to remove multiple image properties)' @@ -974,22 +993,22 @@ class UnsetVolume(command.Command): volume = utils.find_resource(volume_client.volumes, parsed_args.volume) result = 0 - if parsed_args.property: + if parsed_args.properties: try: volume_client.volumes.delete_metadata( - volume.id, parsed_args.property + volume.id, parsed_args.properties ) except Exception as e: - LOG.error(_("Failed to unset volume property: %s"), e) + LOG.error(_("Failed to unset volume properties: %s"), e) result += 1 - if parsed_args.image_property: + if parsed_args.image_properties: try: volume_client.volumes.delete_image_metadata( - volume.id, parsed_args.image_property + volume.id, parsed_args.image_properties ) except Exception as e: - LOG.error(_("Failed to unset image property: %s"), e) + LOG.error(_("Failed to unset image properties: %s"), e) result += 1 if result > 0: